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

Wiktionary translations can't be shown #1033

Open
ghost opened this issue Feb 23, 2020 · 45 comments · Fixed by #1701
Open

Wiktionary translations can't be shown #1033

ghost opened this issue Feb 23, 2020 · 45 comments · Fixed by #1701
Assignees
Labels
bug good first issue regression First as tragedy, then as farce ;-) wikimedia Direct impact on Wikimedia content scraping
Milestone

Comments

@ghost
Copy link

ghost commented Feb 23, 2020

@danielzgtg commented on Feb 23, 2020, 3:21 AM UTC:

Describe the bug

The translations in Wiktionary cannot be viewed anymore on the latest ZIMs. They start collapsed and do not expand when tapped. This applies to translations, conjugations, and collapsible tables in general.

This happens with:

  • wiktionary_en_all_maxi_2020-01.zim
  • wiktionary_en_all_nopic_2019-12.zim
  • wiktionary_en_all_novid_2018-06.zim

This does not happen with and things worked fine with wiktionary_en_all_nopic_2017-08.zim

Expected behavior

To be able to see the translations.

Steps to reproduce the behavior:

  1. Tap on "Library" in the overflow menu
  2. Open wiktionary_en_all_maxi_2020-01.zim
  3. Open the definition for "wiki"

Screenshots

Actual behavior:

Screenshot_20200222-191642

Example of expected behavior in older ZIM:

Screenshot_20200222-191715

Environment

  • Version of Kiwix Android : 3.1.3
  • Device : OnePlus 7 Pro
  • OS version : OxygenOS Open Beta 9

This issue was moved by kelson42 from kiwix/kiwix-android#1796.

@ghost
Copy link
Author

ghost commented Feb 23, 2020

@kelson42 commented on Feb 23, 2020, 7:14 PM UTC:

@danielzgtg Thank you for your bug report, this is a bug in the content itself, moving the issue to the right repository.

@kelson42 kelson42 added bug wikimedia Direct impact on Wikimedia content scraping good first issue labels Feb 23, 2020
@LakmaNeha
Copy link

@kelson42 I am interested in taking up this issue. Can I work on this?

@kelson42
Copy link
Collaborator

@LakmaNeha Thx, would be great

@LakmaNeha
Copy link

@kelson42 This issue is not only with translations, but also affects all the sections which uses navFrames. Below image is one of affected section
image
After some digging, I found out that the toggle behaviour of the navFrames is served by the extension ext.gadget.defaultVisibilityToggles(link). Although If you observe the startup script, the defaultVisibilityToggles extension is being registered. I doubt that the extension is being registered or not for whatever reason, because the event handlers for navFrames > navHead are not being attached, hence the unexpected behaviour. Can you give me some pointers on how to debug this?

Thanks.

@kelson42
Copy link
Collaborator

kelson42 commented Mar 7, 2020

@LakmaNeha Thank you very much for your last comment, we will try to fix the problem rapidly.

@Jaifroid
Copy link
Collaborator

Jaifroid commented Mar 7, 2020

Just to add that this seems to be a CSS issue. Kiwix JS Windows, in its standard mode (which applies its own locally cached copy of Wikimedia CSS, not always 100% updated) is able to display these translations and inflections. But when the setting to use the CSS from the ZIM is applied, it is no longer able to display them. Please note that Kiwix JS Windows does not run JavaScript from the ZIM, which is what leads me to believe that the issue is with the CSS.

A rule needs to be added to the override CSS provided by mwoffliner to display these sections in an initially open state. In general, all sections should always be initially open, and then they should be closed by JavaScript. This ensures that clients that do not execute JS-in-the-ZIM can always display all the information on a page. See #962 for extensive discussion of a closely related issue.

@LakmaNeha
Copy link

Please note that Kiwix JS Windows does not run JavaScript from the ZIM, which is what leads me to believe that the issue is with the CSS.

Hi @Jaifroid , Thanks for the response. First of all, sorry for not being clear about the environment. I am using Kiwix for macOS, Is it the same for kiwix for macOS as well?

Secondly,

A rule needs to be added to the override CSS provided by mwoffliner to display these sections in an initially open state.

Inspecting the site css

image
I found out that there is already a rule existing which makes the sections visible when JS is not supported and applies display: none property if the JS is supported. You can verify this by disabling the javascript and reload the page wiki page, you will observe by default all the translation and inflections are open. That is the reason I suspected JS.

@Jaifroid
Copy link
Collaborator

Jaifroid commented Mar 7, 2020

Yes, you're right that there is that rule in the css file with the snappy filename:

-/s/css_modules/skins.minerva.base.reset|skins.minerva.content.styles|ext.cite.style|site.styles|mobile.app.pagestyles.android|mediawiki.page.gallery.styles|mediawiki.skinning.content.parsoid.css

But the .client-js class has not been added by JS, it is already in the <body> element of the html file:

image

The class .NavFrame is on the parent div, and the class .NavContent is on the hidden div, with the result that the div containing the translations table has display: none:

image

As I say, this is in Kiwix JS Windows, with the client set to use the CSS from the ZIM, and without executing any JavaScript in the ZIM. Therefore, it is definitely not JS that is hiding the div, it is the CSS rule you have found. The confusion here, I think, is that you perfectly reasonably assume (from the name) that the .client-js class has been added to the <body> dynamically, but in fact it is already in the raw HTML that comes straight from the ZIM.

@Jaifroid
Copy link
Collaborator

Jaifroid commented Mar 7, 2020

A fix for this would be to change the CSS rule in that file to:

image

i.e., add details:not([open]) to that rule. The translation table would then be visible until the JS executed onload removes the open, which is how all the other details-summary tags work.

@kelson42
Copy link
Collaborator

kelson42 commented Mar 8, 2020

The main question for this kind of bug, is "Why it does not work out-of-the-box". MWoffliner is conceived to mirror js_modules, css, jss and use the proper js_modules in each page... so what is done wrong here?

@Jaifroid
Copy link
Collaborator

Jaifroid commented Mar 8, 2020

@kelson42 The code for opening and closing sections is manipulated by mwoffliner, and the history of this is quite long. See #633, #677 and #962. Mwoffliner wraps details - summary tags around content that should be opened and closed with a click, adds a polyfill for browsers that do not support details-summary, and adds its own JavaScript for closing all sections when the page is first loaded. In principle, all sections should be open until the JS specifically closes headings other than the top-level heading. However, in the case of the Translations and Inflection sections of Wiktionary pages, the CSS rule cited above is hiding the sections despite the fact that the details tag wrapped around it is clearly marked as "open":

image

So we need to override that CSS rule, so that the section is visible until our inserted JS hides it by removing the "open" attribute. The best place to do that is probably not in the file I mention above, but in the custom override CSS file that mwoffliner adds (maybe it's called "inserted_style.css" or "inserted_style_mobile.css").

@LakmaNeha
Copy link

@Jaifroid so instead of fixing this by overriding a CSS rule, shouldn't we fix the root cause here, which is: "Why client-js class has been added already in the zim file?" which may raise some other problems as well. Ideally by default, client-nojs must be added to the body, and replacing it by client-js or leaving it as it is must be taken care by the JS. That way we provide the default behaviour even if JS is not supported.

But it is done exactly in the reverse way: client-js is replaced by client-nojs in the Startup script code which replaces client-js with client-nojs if JS is not supported.

image

What do you think?

@Jaifroid
Copy link
Collaborator

Jaifroid commented Mar 8, 2020

@LakmaNeha Yes, that sounds like it could work, though it would need careful testing for unintended consequences. I think just removing client-js would deactivate a number of the hiding rules, and then the startup script can add client-js if the reader supports JS.

@stale
Copy link

stale bot commented Jun 14, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@stale stale bot removed the stale label Feb 8, 2022
@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@ynikitenko
Copy link

Many thank for this fix! It was the major software gifts to me before the New Year!

However, when I just bought a new Android tablet this morning, I downloaded Kiwix 3.4.5 and the most recent English wiktionary (from 29.09.2022, with pictures). Translations are still not shown, unfortunately! However, I can see them in a German Wiktionary with pictures from 2023-01-18.

Maybe the recent English Wiktionary data file should be updated for that?..

@kelson42
Copy link
Collaborator

kelson42 commented Feb 3, 2023

We will make a new release of MWoffliner this week and then new ZIM files will have to be created with it...

@danielzgtg
Copy link

By now, I barely use Kiwix Android. The old Kiwix Linux desktop is deprecated and doesn't have zstd, and the new one conflicts with many parts of my system setup. I don't use Chrome anymore thus Kiwix JS Chrome after I switched over to Firefox.

Kiwix JS Firefox is perfect. It opens a service worker webpage. It even supports the Dark Reader extension which looks better than the built-in themes. Other extensions like Tampermonkey are supported as well and I could have used it to inject a style to show the translations. It does have a glitch with network/history but oh well I'll just refresh. But these days, I just use F12 in Firefox as a workaround to show the translations when I need them.

@danielzgtg
Copy link

Here is a Tampermonkey userscript (only kiwix-js that is in ServiceWorker mode is supported) to work around this for older zims:

// ==UserScript==
// @name         Kiwix unhide
// @namespace    http://tampermonkey.net/
// @version      0.1
// @description  try to take over the world!
// @author       You
// @match        https://moz-extension.kiwix.org/*
// @grant GM_addStyle
// ==/UserScript==

(function() {
    'use strict';
GM_addStyle(`
.NavContent {
display: block !important;
}
`);
})();

@kelson42
Copy link
Collaborator

kelson42 commented Mar 4, 2023

@danielzgtg I have no clue neither what you comment means nor why you keep commenting on a closed ticket. If anything is still not ok with mwoffliner, please open a new ticket.

@danielzgtg
Copy link

That comment was just a workaround for people that are using outdated zims. Newer versions of mwoffliner are fine, I just thought that snippet would be useful while people wait.

@Jaifroid Jaifroid reopened this Jul 14, 2023
@Jaifroid
Copy link
Collaborator

Jaifroid commented Jul 14, 2023

According to a user on Reddit, and I corroborate, there is a regression with this issue in wiktionary_en_all_maxi_2023-06.zim. Or the issue may be with Kiwix Desktop (version x64_2.3.1-2, latest available in release directory). The translations show fine in in Kiwix JS PWA. See screenshots below of the article "morning" (with a small m) in both Kiwix Desktop (top) and the PWA (bottom). Translations can't be opened in Kiwix Desktop.

image

image

@Jaifroid Jaifroid added the regression First as tragedy, then as farce ;-) label Jul 14, 2023
@kelson42 kelson42 modified the milestones: 1.12.0, 1.15.0 Sep 30, 2023
@ewtoombs
Copy link

ewtoombs commented Oct 9, 2023

It is still broken in wiktionary_en_all_nopic_2023-07, as served on library.kiwix.org and on my local kiwix-serve instance.

https://library.kiwix.org/viewer#wiktionary_en_all_nopic_2023-07/A/chair

@ewtoombs
Copy link

ewtoombs commented Oct 10, 2023

Here is a more targetted workaround:

// ==UserScript==
// @name         kiwix-wiktionary-toggle-navcontent
// @version      0.0
// @description  Fixes toggling of all NavContent instances in kiwix wiktionary.
// @include      http://localhost:1024/*
// ==/UserScript==

for (nh of document.getElementsByClassName("NavHead")) {
  nh.onclick = (e) => {
    ncs = e.target.parentElement.getElementsByClassName("NavContent")[0].style;
    if (ncs.display == "block") {
      ncs.display = "none";
    }
    else {
      ncs.display = "block";
    }
  };
}

This re-enables the original buttons being used to toggle the translations. It assumes kiwix-serve is running on port 1024. It is only tested on the hidden translations.

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