Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More reliable loading of remote thumbnails #1036

Merged
merged 6 commits into from
Feb 14, 2024

Conversation

veloman-yunkan
Copy link
Collaborator

In the wake of #1035 comes the next small PR.

It solves two problems:

  1. Loading of thumbnails if a debug server (support for which was introduced in the first commit of A couple of bugfixes in Download management #1024) is used
  2. Changing the library view (applying filtering, switching from online to local) while the loading of thumbnails is in progress. I don't have a test case proving that it can lead to some undesirable behaviour. I tried the simplest scenario but it worked nicely. It's possible that no such scenario exists since the potential bug may be compensated by extra code in other places. However these changes make the implementation more robust.

When using a debug server (enabled by the environment variables
KIWIX_CATALOG_HOST and KIWIX_CATALOG_PORT) the URLs of the favicons
must be composed correspondingly.
Names should reflect the semantics rather than the anatomy.
Since the content manager model may be updated (e.g. by applying a
filter) while the thumbnails/favicons are still being loaded it is
better to use a model invariant id for the thumbnails.
src/thumbnaildownloader.h Outdated Show resolved Hide resolved
src/thumbnaildownloader.h Show resolved Hide resolved
@mgautierfr mgautierfr merged commit cc159a2 into main Feb 14, 2024
4 checks passed
@mgautierfr mgautierfr deleted the more_reliable_thumbnail_loading branch February 14, 2024 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants