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

All main <section> tags are erroneously hidden with the new mobile html endpoint, and so inaccessible to readers that cannot run JS #2074

Closed
Jaifroid opened this issue Aug 4, 2024 · 18 comments
Assignees
Labels
bug regression First as tragedy, then as farce ;-)
Milestone

Comments

@Jaifroid
Copy link
Collaborator

Jaifroid commented Aug 4, 2024

(Originally reported in openzim/zim-requests#1129 (comment))

Possibly as a remnant of the removal of details-summary tags for openeing and closing articles (#1915), all main <section> tags have the attribute style="display: none;" (see screenshot at bottom) in ZIMs scraped with the new mobile html endpoint. There is probably a piece of JS included with the ZIM (a script) that is in charge of removing this attribute (it can be seen when loading a page: the lede loads first, and then after a pause the rest of the article is unhidden). However, this logic is the wrong way round: all sections should be visible by default, and then if the reader wishes to hide a section, it can run said script.

This has a serious consequence in readers that block JS from the ZIM, or which cannot in fact run JS from the ZIM: most content in an article becomes inaccessible and cannot be unhidden. This is the case of Restricted Mode (aka JQuery Mode) in Kiwix JS and Kiwix PWA.

For relevant older issues, see #962, #838, #952, (related) #1033, and #1915.

image

@audiodude
Copy link
Member

@Jaifroid So I should be able to confirm this is not an issue with 1.14 by simply running a scraper and looking at the HTML in the ZIM? If I don't see the display: none it's fixed?

@audiodude
Copy link
Member

@Jaifroid So I should be able to confirm this is not an issue with 1.14 by simply running a scraper and looking at the HTML in the ZIM? If I don't see the display: none it's fixed?

@Jaifroid Sorry, I misread the linked issue, this is already confirmed to be happening in a ZIM created with 1.14 yes?

@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Aug 4, 2024

Yes, it was with a Wikivoyage ZIM @kelson42 ran for me yesterday. Let me check the meta data...

@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Aug 4, 2024

Well, the metadata of this 2024-08 English Wikivoyage say 1.13, but it must be using the new endpoint because all the stylesheets are different, and it's a very different ZIM from the last available version (2024-06)... And having the sections all force-hidden was not an issue with all previous Wikivoyage scrapes.

image

@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Aug 4, 2024

Issue also manifesting in the Chrome extension in ServiceWorkerLocal mode (as well as in Restricted mode). In SW-local mode, inline JS is blocked by the Chrome extension APIs, and so the following script in the HTML of each page is blocked:

image

(Not 100% sure that's the issue, but it's the only inline script I've found.) This inline script must be calling a function defined in an attached script, and that function probably traverses the DOM for hidden sections and unhides them. (Remember also that attached scripts won't run in the Browser Extensions in Restricted mode either.)

This behaviour will be in the code coming from the endpoint, I doubt it's been introduced by someone in mwOffliner, though it's possible (and has happened in the past). All that's needed is code in mwOffliner to remove the style display rule on these nodes before the HTML is stored in the ZIM, assuming we always want all sections to be unhidden by default, and assuming we don't want to replicate the old click action which opened and closed sections using details-summary tags.

@THEBOSS0369
Copy link

Hey @Jaifroid @audiodude ! I think i can work on this issue but before i come with up with any approach, i was trying to setup this repo locally and i'm getting this error while running npm ci i have switched to node v18.20.4 using nvm use 18 . I was following contributing.md file
this is the error i'm getting
Image
Image

Thanks!

@Jaifroid
Copy link
Collaborator Author

@THEBOSS0369 Although I opened this issue, mwOffliner isn't my Repo, so I don't unfortunately know anything about the npm errors you are experiencing.

I just checked the latest 2024-10 ZIM of Wikivoyage that @audiodude ran for me the other day, and this issue seems no longer to be present. @audiodude Can you confirm that you fixed it and this issue can be closed?

@audiodude
Copy link
Member

@THEBOSS0369 see #2029

@audiodude
Copy link
Member

I just checked the latest 2024-10 ZIM of Wikivoyage that @audiodude ran for me the other day, and this issue seems no longer to be present. @audiodude Can you confirm that you fixed it and this issue can be closed?

No I didn't fix it, I would have linked to this issue if I did. Possibly the output of mediawiki changed?

@Jaifroid
Copy link
Collaborator Author

No I didn't fix it, I would have linked to this issue if I did. Possibly the output of mediawiki changed?

Yes, I was sure you would have! So maybe fixed at source. I think we should verify rather than go on one ZIM, if we have any ZIMs other than Wikivoyage currently produced from dev.

@THEBOSS0369
Copy link

@audiodude ! Can you look into this i have fixed the npm ci error and shared the steps for it here #2029 but while trying to start the server this error is coming i tried to solve it but ain't to able to resolve it .
Image

If you have any solution for this it will be very helpful
Also do i have to add the email everytime or i can leave it?

@THEBOSS0369
Copy link

Hey @audiodude ! can you guide me how to fix this error i haven't been able to fix this yet i tried it from stack overflow and chat gpt but still ain't able to fix this. Your help will be appreciated. Thanks!

@audiodude
Copy link
Member

Have you tried the command listed in the README? npm run mwoffliner

@audiodude
Copy link
Member

@Jaifroid Are these the section tags that had display: none?

Image

This is with a ZIM I created with 1.14 a while ago, wikipedia_bm_all_maxi_2024-08.114.zim.

Image

@Jaifroid
Copy link
Collaborator Author

@audiodude I'm guessing your snapshot is taken after the included JS is run to remove style="dispay: none;. It certainly looks like something has removed whatever was in the style attributes and has just left empty attributes. My point is about the raw HTML before any client-side manipulation of it has occurred.

Looking at the latest wikipedia_bm_all_nopic_2024-10.zim scraped with 1.14.0-dev, the issue is still present. E.g. in the "Mali" article, you can see all subsequent sections after section 0 are hidden in the raw HTML, and the article is visually missing all the detail thereafter:

Image

You can test this in the Kiwix JS browser extension (available from the Chrome or Firefox Add-On stores). This will initially offer to open the ZIM in restricted mode for security reasons. Accept this, and then the issue can easily be seen in that small ZIM.

My point is that the logic needs to be reversed: the style="none;" declarations should be removed at scrape time, so they are not in the ZIM. The current functionality is useless in any case, because headings can't be clicked to open or close them even in ServiceWorker mode (or in any ZIM reader that runs client-side JS from the ZIM). If we want to implement that functionality, we need to add our own summary-details tags at scrape time, which was the previous solution, or implement that in CSS only. If we have decided to go with all headings always open even on mobile devices (a decision I wouldn't disagree with, especially when all readers have an easily accessible Table of Contents for ZIM articles), then I would suggest removing the style="display: none;" attributes at scrape time and also the redundant script that unhides sections client-side.

@Jaifroid
Copy link
Collaborator Author

I've opened a separate issue for the problem mentioned in #2074 (comment). I confirm this exists in the latest wikipedia_bm_all_nopic_2024-10.zim.

@THEBOSS0369
Copy link

Have you tried the command listed in the README? npm run mwoffliner

Yes sir I tried, it wasn't working I found that this is happening due to not starting of redis server which was happening because GitHub codespaces wasn't allowing me. I will try to set up repo locally but not sure I will be able to do that due to less space in my system 😓

@audiodude audiodude self-assigned this Nov 4, 2024
@audiodude
Copy link
Member

Fixed by #2097

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug regression First as tragedy, then as farce ;-)
Projects
None yet
Development

No branches or pull requests

4 participants