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

ZIM metadata „illustration“ is not read properly #789

Closed
kelson42 opened this issue Jun 1, 2024 · 17 comments · Fixed by #811
Closed

ZIM metadata „illustration“ is not read properly #789

kelson42 opened this issue Jun 1, 2024 · 17 comments · Fixed by #811
Assignees
Milestone

Comments

@kelson42
Copy link
Contributor

kelson42 commented Jun 1, 2024

Here we have a proper logo (with Kiwix Server)
https://dev.library.kiwix.org/#lang=&q=Mali+mp4

But the same file in the Kiwix local library displays the defaukt logo placeholder
image

@kelson42 kelson42 added the bug label Jun 1, 2024
@kelson42 kelson42 added this to the 3.4.0 milestone Jun 1, 2024
@kelson42
Copy link
Contributor Author

kelson42 commented Jun 1, 2024

Not sure how this is done, but there is a primitive in libkiwix which shiukd be used:
https://libkiwix.readthedocs.io/en/latest/api/classkiwix_1_1Book_1_1Illustration.html?highlight=Illustration

I suspect that because the logo of the ZIM is accessed directly and because we have changed in the path the URL, then something is wrong. Please read https://wiki.openzim.org/wiki/Metadata

@rgaudin
Copy link
Member

rgaudin commented Jun 14, 2024

The reader is using its url property

book->getIllustrations().at(0)->url

Documentation is of no help here but a quick glimpse at the code have me believe that url is only set when read from XML or from OPDS so in apple reader's case, only if downloaded from the catalog. I could confirm this with a test.

@BPerlakiH, we should use getData() indeed.
A quick hack could be returning a base64 image/png url as this is used as an URL 😝

@kelson42
Copy link
Contributor Author

Makes sense, metadata should be retrieved from online catalogue AFAP, but if no entry there, ALL metadata (illustration included) should come from the ZIM file.

@kelson42
Copy link
Contributor Author

Still not working:
image

@kelson42 kelson42 reopened this Jun 26, 2024
@kelson42
Copy link
Contributor Author

After iOS update to version 18... it works!
image

I'm a bit puzzled.

@rgaudin
Copy link
Member

rgaudin commented Jun 27, 2024

Solar Powered and Quartiers chinois still use category image in this screenshot…

@rgaudin
Copy link
Member

rgaudin commented Jun 27, 2024

Given you use this device to test the app, I'm not sure it's a wise choice to be on a beta system… but there's no downgrade possible so it's too late now 😀

@kelson42
Copy link
Contributor Author

kelson42 commented Jun 27, 2024

Given you use this device to test the app, I'm not sure it's a wise choice to be on a beta system… but there's no downgrade possible so it's too late now 😀

Yes, but see it the other way: we prefer to detect problems with iOS18 earlier, better than waiting the system is live.

@rgaudin How does you iOS device behaves regarding this very specific issue? Is rhat fixed for you?

@rgaudin
Copy link
Member

rgaudin commented Jun 27, 2024

I figured, but you're the only tester we have! Hope it works well on current stable!

@rgaudin
Copy link
Member

rgaudin commented Jun 27, 2024

Current Testflight for macOS (published 3days ago) still exhibits the bug

Screenshot 2024-06-27 at 16 22 23

@kelson42 kelson42 reopened this Jun 27, 2024
@kelson42
Copy link
Contributor Author

@rgaudin Thx, I wonder if there is not a cache somewhere which would make the test of the patch complicated!?

@BPerlakiH
Copy link
Collaborator

Current Testflight for macOS (published 3days ago) still exhibits the bug

Screenshot 2024-06-27 at 16 22 23

@rgaudin can you tell me which icon is wrong ?

@rgaudin
Copy link
Member

rgaudin commented Jun 28, 2024

The one you see top left. that's not a zim icon but the other category icon in the app.

Only zim added via catalog shows their icons

@BPerlakiH
Copy link
Collaborator

BPerlakiH commented Jun 28, 2024

I think I have the same file, on the current main branch it looks good after adding it.
Screenshot 2024-06-29 at 00 40 53

I have the same result using the TestFlight version.
The code change itself won't change the image data already written into the local DB, before the fix.
Can you try to unlink the file, and add it again?

@BPerlakiH
Copy link
Collaborator

I think the same happened for @kelson42 as well, after the iOS update, the files must have been re-added to the local DB, and from then on they were fixed.

@kelson42
Copy link
Contributor Author

Moving to 3.5.0

@kelson42 kelson42 modified the milestones: 3.4.0, 3.5.0, 3.4.1 Jun 29, 2024
@rgaudin
Copy link
Member

rgaudin commented Jun 30, 2024

The code change itself won't change the image data already written into the local DB, before the fix.

OK that explains it. Are we OK with this? If yes then this ticket should be closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants