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

Remove/refactor code in zim_prefix.js that still tests for a favicon with I/ namespace in no-namespace ZIMs #139

Closed
Jaifroid opened this issue Sep 28, 2022 · 3 comments · Fixed by #140
Labels

Comments

@Jaifroid
Copy link

The code in zim_prefix.js#L3 in this repo should probably have been removed or refactored when the scraper was upgraded to produce C-namespace / "no namespace" ZIMs, but unfortunately it continues to look for a favicon with an I/ namespace. See screenshot of code below in a recent Type 1 ZIM, ted_en_playlist-animals-that-amaze_2022-08.zim'. It produces side effects in my reader, which I have now worked around, but I thought the issue should be documented at source.

Further info: The bugs that I've been experiencing related to this issue here: kiwix/kiwix-js-pwa#297, and documented fairly fully here: openzim/nautilus#40, are caused by an interaction between the Kiwix JS PWA reader and code that attempts to read for the location of a favicon in YouTube, TED and Nautilus scrapers. The code does return a namespace of "" in a no-namespace ZIM (if it runs), but this is a side-effect and probably not even needed any more. The problem for my code was that if there was no favicon, then the code would bug out instead of gracefully returning null, causing a JS exception which prevented display of assets.

image

ZIM DirListing version: 1 ZIMFile {_files: Array(1), name: 'ted_en_playlist-animals-that-amaze_2022-08.zim', id: 4, majorVersion: 6, minorVersion: 1, …}

@rgaudin
Copy link
Member

rgaudin commented Sep 29, 2022

@Jaifroid the scraper was never adapted to no-NS but another bug was fixed by upgrading scraperlib which resulted in created no-NS ZIMs but keeping that namespace code…
Thanks for reporting. Can you confirm that https://mirror.download.kiwix.org/zim/.hidden/dev/ted_en_playlist-what-robot-can-teach-us_2022-09.zim is fine?

@Jaifroid
Copy link
Author

@rgaudin I confirm that the ZIM you linked to fixes this issue! It no longer has the problematic script in it, and an unpatched version of the KJSW app can read it without error. Many thanks.

@rgaudin
Copy link
Member

rgaudin commented Sep 30, 2022

Great; will make a release then

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

Successfully merging a pull request may close this issue.

3 participants