The assertion failure can be triggered by full-text-searching for a
common word, selecting a result with many large articles and quickly
selecting text in the first article while the page is still loading.
This reverts commit c936d03fa0.
A default-constructed QRegExp is passed to ArticleView::showDefinition()
when a user navigates from an FTS-result page by clicking a link or
double-clicking a word. QRegExp().pattern() returns an empty string,
which is stored in the "regexp" query item of an URL. When this URL is
loaded, ArticleView::loadFinished() calls highlightFTSResults(), which
then calls closeSearch(), performs multiple string and regular
expression operations and early-returns because the regular expression
pattern is empty. Returning earlier skips useless work in this case.
An alternative optimization is not calling highlightFTSResults() at all
when the regular expression is empty, but that would skip the
closeSearch() call and keep the FTS search frame visible on a page with
an empty regexp.
One issue with the current implementation is that a wrong-cased match
remains selected when the user checks the Case Sensitive checkbox.
Another issue is that the noResults property of searchText is not
updated right away, which means that searchText's background color
remains wrong until next search.
Handle toggling case sensitivity in the same way as editing the searched
text: restart search from the beginning of the page. This improves the
Search-in-page UI consistency.
The current article is saved to pages history user data before any
navigation to another page and before reloading the current page.
Therefore saving it each time the user activates an article is
redundant. This redundancy is not consistent, because when a user
activates an article by clicking on it, the current article changes in
the UI, but is not immediately saved in history as setCurrentArticle()
is not called. The inconsistent redundancy is a waste of CPU time and
can hide bugs.
This variable overrides history user data, which makes the current
article and position restoring code in ArticleView::loadFinished()
difficult to understand.
Encode the same logic in the history user data instead. This should make
the code more straightforward and less brittle in the face of changes.
MainWindow calls ArticleView::reload() when the group list is updated.
This updating may add/remove/reorder dictionaries in the active group.
MainWindow also calls ArticleView::reload() when the display or addon
style changes.
In both of the above scenarios uncontrolled jumps and current article
change can occur (see also the parent commit message).
Move setting articleToJump from above the only ArticleView's reload()
call into ArticleView::reload() itself.
When the first article in the list is current, expanding or collapsing
optional parts results in:
1) uncontrolled jumps due to the content height changes if the scroll
position is not (0, 0);
2) current article change if a non-first article is saved in history
user data (e.g. if a mouse click had made the first article active).
Treat the first current article in the same way as non-first ones.
This change also affects the case when the current article is not
present in the article list. If the current article is simply empty,
then there is no behavior change. If the current article is not empty,
it *must* be in the list, unless something else went wrong.
This way it is clearer that the pages history is updated just before
each navigation to a different page.
The call to ArticleView::saveHistoryUserData() now occurs slightly later
in ArticleView::showDefinition(). I don't think the intervening code can
affect the current article or window position. So the reordering most
likely does not affect application behavior.
Without the added saveHistoryUserData() call, returning back from the
enlarged picture page ("gdpicture") restores the current article and the
window position that were last saved. At this commit the "gdpicture"
behavior is consistent with regular links: returning back from the
enlarged picture page sets the article with the picture as current and
restores the window position at the time of the click on the picture.
https://doc.qt.io/archives/qt-5.5/qtwebkit-bridge.html#internet-security
Qt WebKit Bridge documentation recommends:
When exposing native objects to an open web environment, it is
important to understand the security implications. Think whether the
exposed object enables the web environment access things that
shouldn't be open, and whether the web content loaded by that web page
comes from a trusted source.
The author of Qt WebChannel has said the following in a talk that
introduced this Qt module (WebKit Bridge replacement for Qt WebEngine):
My suggestion here is to write dedicated QObjects with a slim, minimal
API that only have the signals and methods that you deem safe to be
used from the outside.
- see a comment under https://redirect.invidious.io/watch?v=KnvnTi6XafA
When the current article is set and the user expands or collapses
optional parts (e.g. via the Ctrl+* shortcut),
ArticleView::setCurrentArticle() is called twice from
ArticleView::loadFinished(). Furthermore, the window scroll position is
restored before the second jump. This is wasteful. Move the
higher-priority setCurrentArticle() call up and, if it succeeds, skip
the other call and the scrolling.
I have measured the time spent running the affected code fragment on my
GNU/Linux system before and at this commit. When the loaded articles are
not very large, the performance gain of this commit is only about 1 ms.
However, when one of the displayed articles was huge (the
"United States" English Wikipedia article), the time went from 120 ms to
5 ms.
The wall time of calls to ArticleView::highlightAllFtsOccurences() on my
GNU/Linux system before and at this commit:
allMatches.size() uniqueMatches.size() before(ms) at(ms)
79 1 277 4
98 1 380 4
267 1 16803 65
Extracting this function allows to simplify the code and facilitates
optimizing it in the next commit.
Remove `#if QT_VERSION >= 0x040600` along the way as GoldenDict does not
support Qt versions older than 4.6 for several years now.
Preferences::sanitizeInputPhrase() transforms an input phrase by
removing its whitespace/punctuation prefix and suffix. Translating a
phrase from X11 primary selection or from clipboard, via mouse-over or
from the command line results in such sanitization. This is useful when
a punctuation mark or a space is selected accidentally alongside a word.
This sanitization can be undesirable, however, when an abbreviated word
is selected. For example: "etc.", "e.g.", "i.e.".
This commit implements searching for the input word with the punctuation
suffix preserved as an alternative form of the sanitized word to show
articles for both. For example, when the word "etc." is translated from
the clipboard, both "ETC" and "etc." articles are displayed.
The punctuation suffix is preserved when the word is passed from the
scan popup to the main window and when the translate line text is
refreshed (e.g. when the current group is changed). The suffix is not
stored in history and favorites (doing so would require file format
changes and possibly substantial code changes, this can be implemented
later if need be).
Trim the input phrase once in ArticleNetworkAccessManager::getResource()
instead of verbose trimming in multiple places in
ArticleMaker::makeDefinitionFor().
Closes #1350.
For example, the first audio link in "The United States" English
Wikipedia article - "The Star-Spangled Banner" - ends with ".oga".
Without this commit the audio link is not recognized by GoldenDict:
* it is not pronounced when a Preferences=>Audio=>"Auto-pronounce..."
option is enabled;
* clicking on the link opens it in the default browser instead of
playing inside GoldenDict.
When a long text is accidentally selected or copied while the scan popup
is on, Goldendict spends a lot of CPU time to gradually create a
"No translation..." article. When translation of huge (e.g. 15 MB) text
from the clipboard is (accidentally) requested, Goldendict freezes for a
while. Turning the added input phrase limit option on eliminates this
waste of the CPU time.
I have implemented these options primarily for selection and clipboard,
but they also affect mouse-over translation on Windows and command line
translation requests. This is mostly because I did not bother to limit
the options' scope. I guess hovering over an extremely long text without
spaces (e.g. Base64-encoded) could cause the same performance issue on
Windows. The command-line translation could be requested from a script
that integrates Goldendict with some other application, from which long
text could be sent for translation by accident.
I hope that the default value of 200 characters will be sufficient for
just about any real-world user input in any language. The option is on
by default, because the default length limit is generous and any longer
text is unlikely to be sent for translation intentionally. My personal
preference for the input phrase length limit is 100 symbols.
ArticleView::pasteTriggered() didn't call QString::simplified() on the
text retrieved from the clipboard. I assumed this was an oversight, so
now it *is* called - indirectly, via Preferences::sanitizeInputPhrase().
External and internal audio players work similarly now. Fixes #950.
* inherit a new ExternalAudioPlayer class from AudioPlayerInterface;
* use an existing ExternalViewer class to implement ExternalAudioPlayer;
* take (const char *, int) instead of std::vector<char> in
ExternalViewer constructor to fit into AudioPlayerInterface;
* extend ExternalViewer API to let ExternalAudioPlayer stop superseded
audio player processes;
* make AudioPlayerInterface::play() return an error message string to
allow reporting immediate failures from derived classes;
* Document AudioPlayerInterface API;
* Document AudioPlayerFactory::player();
* use the common audio interface exclusively in ArticleView.
* add a new interface class AudioPlayerInterface;
* inherit a new proxy class Ffmpeg::AudioPlayer from it;
* partially switch ArlticleView to using the interface;
* expose MainWindow's AudioPlayerInterface instance to all ArticleView
instances;
* add a new AudioPlayerFactory class responsible for creating instances
of concrete classes derived from AudioPlayerInterface depending on
relevant Config::Preferences values;
* increase minimum supported Qt version from 4.5 to 4.6 in README
in order to use QScopedPointer introduced in Qt 4.6.
Internal links in MediaWiki articles don't lead to word definitions,
so the "Definition: " prefix is not correct for them. It also does not
provide any information and is largely useless.
Perhaps the actual footnote text should be displayed in the tooltip,
but this requires distinguishing them from citations/references and
backlinks.
