Monitoring of the download dir interferes with the download lifecycle
management. Here are some of the observed effects:
- As soon as the ZIM file is created in the download directory it
is added to the library and overwrites the entry created by download
management logic, thus resetting the download id associated with the book.
- As a result `ContentManager::downloadCompleted()` is never called;
`ContentManager::downloadCancelled()` is called instead (however, for
some reason the user-observable effect is as if the download completion
is normally detected and handled). Looks like this was the main
culprit behind the false bug report kiwix/libkiwix#1049.
- The icon of the book added a time when the file was not yet a valid
ZIM file stays invalid.
I am temporarily disabling this functionality so that the work on
download related code is carried out in a clean room environment.
Once it's complete monitoring of the download directory must be
re-enabled but in a way that doesn't confuse the logic of the
download state machine.
Notes:
- The `getAction()` helper function was introduced in order to keep the
line length within the 80 chars limit and was applied to other usages
of `KiwixApp::getAction()` too.
- The tooltip includes the keyboard shortcut which closes only the
current tab but the tooltip is the same for all the tabs.
Obtaining the value of the ALT+[0-9] shortcut from the action objects
would be justified if they were connected to the same (shared) handler.
But since handlers are distinct lambda objects the sought value can be
obtained via a captured variable.
Before this change, after a tab was closed its adjacent tab (the next
one or, in the absence of such, the previous one) became active, no
matter which tab was active before that. Now that logic applies only
if the currently active tab is closed.
The function responsible for that logic was removed because its name
was not a good one and its existence was not justified given that it
could be replaced by a oneliner and that it was used only once.
Before this change, an attempt to close the library tab via a
middle-click - although prevented by a dedicated check - had a
side-effect of switching to the tab next to the library tab.
The only justified check for the availability of download functionality
is in the initiation of the download action
(`ContentManager::downloadBook()`) - if no download is started then
(assuming otherwise correct code) it should be impossible to pause,
resume or cancel one.
One small difference of getFaviconUrl() from the piece of code it was
created from is that, in case the book doesn't contain an illustration,
an empty string is returned directly rather than via a confusing
default-constructed Illustration object.
The commit "Moved download deregistration to ContentManager" introduced
a bug - completed downloads were not properly deregistered.
Note that a simpler fix by adding a single `m_downloads.remove(bookId);`
line in `ContentManager::downloadCompleted()` didn't work (which means
that that function is not always called upon download completion). I am
not going to investigate that issue now, hoping that it will be
automatically resolved once the redesign of download management is
finished.