kiwix / kiwix-js

Fully portable & lightweight ZIM reader in Javascript
https://www.kiwix.org/
GNU General Public License v3.0
299 stars 124 forks source link

In jQuery mode, load the images after CSS (and javascript?) #149

Closed mossroy closed 1 year ago

mossroy commented 8 years ago

So that the CSS stylesheet is applied as soon as possible. Maybe with something like https://stackoverflow.com/questions/2358205/invoking-a-jquery-function-after-each-has-completed

Jaifroid commented 7 years ago

Hi, I'm the person porting this excellent HTML5 app to Windows 10 UWP, targeting Windows Mobile (another minority phone OS, like Firefox OS, so I sympathize....). I am also noticing the issue that with some Wikipedia pages, from a range of ZIM files, the CSS never gets injected due to (I think) a timeout. While it's still possible to read the full text of the page in the browser (and see all images), when this happens, the app is left in an unstable state. It is possible to search for new pages to load, but attempting to load the new page doesn't actually work, and dev tools show it's not just a display issue -- the html of the new page simply fails to load over A.html.

Possibly an asynchronous call is still hanging around waiting for the CSS from the previous page, but that is just a guess. Only solution when this happens is to exit the app and relaunch. It would be interesting to see whether loading the CSS first, before the images, solves the issue.

Additional info: this happens on Windows Mobile, but I have not seen it happen on the non-mobile UWP version of the app. Same codebase, same underlying browser engine (Edge). Yet it doesn't seem to be a memory problem: I've seen heavy pages load the CSS quickly, and I've seen relatively light, text-only pages fail to load the CSS., but I haven't found the cause. As it happened "reliably" with some pages, I'll try to debug this.

Jaifroid commented 7 years ago

OK, so the issue I outlined here is nothing to do with CSS, it's actually a low memory bug. See #253 (EMSCRIPTEN buffer overrun in xzdec.js).

[I did experiment with trying to load CSS before images in the code, but changing the order in the jQuery mode functions does nothing. A more convoluted method like that mentioned in OP is needed.]

Jaifroid commented 7 years ago

@sharun-s , @mossroy , sorry about the hiatus, but I've now finished the code for preloading the stylesheets, and I've submitted it as a branch: https://github.com/kiwix/kiwix-js/tree/Preload-stylesheets .

What it does is to wait for the stylesheet promises to resolve (via callback function) and then create an array of BLOB URLs for them. These are then spliced into the raw HTML before the html is injected into the iframe.

The good news is that we cut down on the redraws, as the stylesheet is loaded at the same time as the HTML is loaded into the iframe, and so picture placeholders are all in the right place already just waiting to be filled. Also, this is done deliberately without jQuery for speed. Regexes are my "speciality"...

The not so good news is that the bottleneck is now proven to be the amount of time it takes to retrieve stylesheets from the ZIM file via the readBinaryFile subroutine. These seem to take a lot longer than images to retrieve, and I'm suspecting that it is down to their location in the ZIM file (after all, the common stylesheets are likely to be stored only once somewhere in the ZIM file, and not necessarily close, in physical terms, to the article).

I think if @sharun-s 's fast image-loading routines could be "generalized" so that they load any file at all, not just images, we might be able to speed up this bottleneck.

In the meantime, I'd be really grateful if you could test (and review) my code, which I've tried to document where it isn't self-explanatory. It's only been lightly tested on a few ZIM articles in Firefox, Edge and Internet Explorer, but I've not noticed any bugs yet.

sharun-s commented 7 years ago

Nice! I took a quick look. I like all the jquery removed. Will the regex handle href followed by rel? Apologies for questioning the regex master ;) Other than that looks good! I will try to use it sometime during the week, to see what it's doing with performance.

These seem to take a lot longer than images to retrieve

Are you seeing the same kind of delay across all browsers? Or it varies? And are you testing on win 10 mobile?

Jaifroid commented 7 years ago

Thanks for the comments, @sharun-s .

Will the regex handle href followed by rel?

Yes, it uses a positive lookahead to check that the <link ...> tag contains rel=stylesheet anywhere before the close > delimiter, and if so attempts to match href="..." anywhere inside the delimiters. Specifically the lookahead is this bit:

   "(?=" +    // Assert that the regex below can be matched starting at this position (positive lookahead)
      "[^>]*" +  // Match any character that is NOT a “>” between zero and unlimited times
      "rel" +     // Match the character string “rel” literally
      "\s*" +       // Match a single character that is a “whitespace character” between zero and unlimited times
      "=\s*" +     // Match the character “=” literally followed by whitespace zero or unlimited times
      '"stylesheet' +   // Match '"stylesheet' literally
   ")"

A lookahead merely asserts that the regex in parentheses can be matched at some point ahead in the string, so allows searches outside of the sequential matching order.

Are you seeing the same kind of delay across all browsers? Or it varies? And are you testing on win 10 mobile?

I've tested on Edge Mobile, and it works but yes the delay is quite noticeable, and there are memory issues generally with this app on large ZIM files with lots of images, which I still need to work on (it's not specific to this code). On Edge PC the delay is acceptable but noticeable. On Firefox, which is fast generally with this app, the delay is very acceptable and hardly noticeable. I haven't tested on Chrome.

sharun-s commented 7 years ago

Interesting didn't realize how the lookahead worked. Good to know!

Also good to know about the delays on Edge. How big is your ZIM file? And is it split?

Jaifroid commented 7 years ago

For Edge Mobile, apart from Ray Charles, I've tested only on unsplit ZIM file of 14.7Gb (Spanish Wikipedia with images) and a no-image version of the English Wikipedia which is 18.7Gb. The no-image one loads significantly faster than the image one, despite larger file size. It's the searching through the file multiple times that demands more and more memory.

Jaifroid commented 7 years ago

I've experimentally added some local caching of common Wikipedia CSS files in the filesystem.

I discovered that these files exist in both the English language and the Spanish language Wikipedia ZIM files with images, and it seems pointless to extract them from the ZIM at great cost in terms of time and memory usage, especially on mobile, when it is just a question of four or five files. See what you think.

NB I know it's not a "universal" or elegant solution, but it sure speeds up the most common use case for kiwix-js, which is displaying articles from Wikipedia... We could quite easily add a number of other use cases, such as wikitravel or Wiktionary, if we support those ZIM files. Of course, we'd need to keep the stylesheets up to date periodically.... NB the stylesheets are stored in "../-/s/*" relative to the A directory, which is the same URL as that stored in the ZIM file. This will make it easy to add further files unless there turn out to be conflicts (same file URL with different content across different Wikis).

@Mossroy, if you don't want to adopt such a solution in kiwix-js, which I'd understand, I would nevertheless implement it in kiwix-js-windows, because on initial testing it significantly reduces the memory footprint for articles from large Wikipedia ZIM files where the CSS seems to be cached at the other end of the file from the article in question. Of course, if we can come up with a better solution through enhanced caching while keeping the memory footprint low, I'd be all for that.

Clearly, this solution needs to be tested against a range of different language ZIM files.

PS Direct link to the commit that has these changes: https://github.com/kiwix/kiwix-js/commit/eca226c831dc6bc6988758dfa8c76e74ac18afde

sharun-s commented 7 years ago

@Jaifroid the issue will be switching back and forth, involving commenting/uncommenting/changing code each time the need arises. Which could turn into a headache.

One option is to add a simple switch (maybe customCSS=true|false) in init.js so that the app and required modules on first load, either use "default" css processing or "custom" css processing. Ideally the loading logic should be in two different functions and at load time the module decides which to use based on config.
Easy to do. For file reading for example, I am using this method to switch between filereader and xhr Here I set the switch value init.js , init.js (use a default value or get it from url/cookie/localstorage/appcache etc) Here I use the switch util.js So when the app loads util.js for the first time only the mode required works. No code changing reqd and no if condition executed each time to decide which code to run.

Lots of stuff can be configured at load time this way. Eventually all these switches/settings can be exposed to the user via the config page.

Jaifroid commented 7 years ago

Yes, I'd be happy to make it user configurable, especially as the same logic can be used for #265 (Provide an optional dark theme both for interface and Wiki articles) and for #34 (On mobile, adapt the layout of articles like on mobile version of online Wikipedia). @sharun-s , is it likely that your web worker code, if applied to CSS retrieval as well as to images, would speed up retrieval enough to make this kind of filesystem caching unnecessary? At the moment I'm viewing this just as an experimental submission for testing the speed differential and memory footprint, particularly on mobile.

sharun-s commented 7 years ago

@Jaifroid well the webworker is only going to retrieve the CSS dirent (not the blob) and from the timing data I have seen CSS dirent retrieval doesn't take any real time to matter much whether it happens on the worker or the main thread.. The delay seems to come from injecting and applying CSS. No testing done but it's probably dependent on the complexity of the CSS and the size of the article. But as you say your change will be a good benchmark to compare against for future CSS loading changes.

Jaifroid commented 7 years ago

One option is to add a simple switch (maybe customCSS=true|false) in init.js so that the app and required modules on first load, either use "default" css processing or "custom" css processing. Ideally the loading logic should be in two different functions and at load time the module decides which to use based on config.

Now implemented in: https://github.com/kiwix/kiwix-js/commit/4ff71cc2ba828e3cdba3cc414bdb7929a62e4404 . The available param values for cssSource are: "zimfile", "local" and "mobile". Thanks for the hints on how to do it @sharun-s . I've included your param for selecting the number of search results, because easy to add into this structure and makes sense to do both at the same time. The one for file reading isn't included yet because that should be separate PR when the code sets are made compatible.

@Mossroy , this commit (https://github.com/kiwix/kiwix-js/commit/4ff71cc2ba828e3cdba3cc414bdb7929a62e4404) goes some way towards implementing #34 (adapt layout of articles like on mobile version of Wikipedia). It doesn't do it in a responsive way, but includes a param which can be modified to set the default source, one of which is "mobile". You can test this now by changing the default in init.js, but we should implement it as a user-selectable choice in the config page.

PS A couple more styles probably need adding to implement the Wikipedia mobile style fully. At the moment it's just one mobile-specific stylesheet.

Jaifroid commented 7 years ago

@sharun-s , @mossroy , Commit https://github.com/kiwix/kiwix-js/commit/2a1ccc0427ac601b37c34716f3772a2bbe455e04 implements a user-selectable display mode (in the user interface). It allows the user to choose between locally cached stylesheets, the Wikipedia mobile display style, or using the styles embedded in the ZIM file. Please have a play in particular with the mobile display style. Here are the Configuration settings:

image

And here is the mobile transformation applied to a Ray Charles page:

image

I've tested in Firefox, Edge PC, Edge Mobile, Internet Explorer 11, and UWP App, against these ZIM files:

wikipedia_es_all_2017-05.zim wikipedia_en_all_2016-12.zimaa (etc.) wikipedia_en_medicine_novid_2017-07.zim (this file already has the mobile style embedded, there is no option to strip the mobile styles out as yet) wikipedia_en_ray_charles_2015-06.zim

sharun-s commented 7 years ago

Nice work @Jaifroid Look forward to playing with it over the weekend when I have more time. How do you test mobile? You mentioned it somewhere before, but I have forgotten. Are you using the Chrome Devtools emulator?
And should have asked this earlier but how did you come up with this list of stylesheets?

Jaifroid commented 7 years ago

@sharun-s , I have a Lumia 950XL for device testing, but I also test it using the Windows Mobile emulator in Visual Studio. Kiwix-js-windows is being developed for UWP on both Windows 10 Mobile (includes tablets) and as a Windows Store app.

I found the stylesheets by capturing them as the app is accessing them and before they are turned into BLOBs (breaking programme flow at that point and examining the variables). It's empirical. There are probably a few more stylesheets, and I need to check on some other languages, though it's not necessary to have a complete set, since the app will access any that are not cached in the fs. For mobile, I've used the wikipedia_en_medicine_novid_2017-07.zim which has been encoded with mobile stylesheets only. You can also get them straight off the Wikipedia mobile website.

sharun-s commented 7 years ago

Got it. Was just curious about the process.

Jaifroid commented 7 years ago

@mossroy , @sharun-s , I'd be really grateful if you could try out the code in the Preload-stylesheets branch. It's quite revealing on the performance issues. I've done major work putting most of the transformations code into a separate module, transformStyles.js, so that it doesn't clutter the app.js code, and the transformations are user-selectable in the interface. In particular, if you leave the "Use locally cached display style" setting enabled, you should notice a large performance difference in the time it takes to first paint and then to subsequent load of images, both in "desktop" display mode and in "mobile" display mode. I'll discuss my findings on the reasons for the huge difference in a different post, especially as I want to attach a timing printout in the console log at the start and end of css retrieval to get some objective figures. The memory usage inside the app version I'm working on is also hugely reduced with this code, and for the first time the app does not start to physically heat my mobile when running either as an app or in the mobile browser.

The transformations between the desktop and the mobile stylesheets work both in the "standard" Wikipedia and Wikivoyage files (I've tested with wikivoyage_en_all_2017-07.zim) that have only desktop styles embedded,, and in the new mw-offliner files such as wikipedia_en_medicine_novid_2017-07.zim . The transformations work in either direction, i.e. you can display a page from this medicine_novid in desktop style even though the ZIM does not contain the desktop stylesheets, and vice versa for the standard ZIMs. This allows you to hide or reveal the extra info at the end of an article, even in the medicine_novid ZIM where all such info is hidden by default (which was actually a problem with that format for anyone wanting to do serious research with the app),.

Obviously this needs lots of testing, and I've done a lot of debugging already and re-organizing the code into a logical flow and tidying it into the transformStyles module, as well as commenting the code extensively.

image

mossroy commented 7 years ago

@Jaifroid I'll have a look at it. I had put this version on my FirefoxOS device (because it's slower, so it's easier to see performance enhancements). But there was no image at all (and no error message), where images seem to work on desktop. I'll try to investigate when I find some time

Jaifroid commented 7 years ago

Thanks for the report, @mossroy . Obviously I can't debug FFOS. Guesses: 1) It could be that this version uses the "oneTimeOnly" parameter to create the blobs, which FFOS might not like. I should revert that here, though will keep for kiwix-js-windows. 2) This version also uses blobs for stylesheets (I could easily revert that part to dumping the stylesheet data into the HTML but in principle blobs use less memory because they're destroyed after first use), but the same issue would affect CSS in that case (oneTimeOnly).

Jaifroid commented 7 years ago

@mossroy, @sharun-s : I've done some performance improvement tests based on the timings I've now put into the console log. These are on Edge and Firefox. The original spreadsheet is here. Below are screenshots. Percentage improvement from click on article directory entry to first paint are: 479% for Edge and 79.9% for Firefox. Not shown here, but also significant, is that the time to load images to completion is also much shorter in both browsers. This is confirming my theory about the problem... (will explain later, after I've done tests on UWP app).

MS Edge:

image

Firefox:

image

Jaifroid commented 7 years ago

@mossroy, @sharun-s: I have pasted the test on UWP mobiles in https://github.com/kiwix/kiwix-js-windows/issues/14 , but the headline gain is 2,288% reduction in time taken from receipt of raw HTML to first paint, a reduction from 31 seconds to 1.3 seconds! Please note that 31 seconds isn't typical: I have deliberately chosen an extremely heavy article, which is very long, has six stylesheets and dozens and dozens of images, some of them little tiny ones, others large, inside a 15.5Gb single ZIM file. This was always going to be pretty taxing for mobile. Note that time to retrieve images is dramatically reduced also (not measured here), which is counter-intuitive, but confirms some things that @sharun-s discovered with his code. Also the app is far more stable (reduced memory requirements).

Why the huge improvement when CSS is just ASCII text and ought to be retrieved instantly? The problem is the asynchronous process! How so? Well, if you open a ZIM file (a small one) with 7-zip or somesuch, you can see that stylesheets are stored once at the front of the file, technically in the "-" namespace, presumably named such so that they would come before any of the other namespaces. Documentation is here: http://www.openzim.org/wiki/ZIM_file_format

This is followed by the "A" namespace where all the HTML articles are. I am not sure whether images are all grouped together in the "I" namespace, or whether the namespace is just a directory device for finding binary pointers to image files grouped together with the articles that use them. Either way, clearly the stylesheets are separated by some distance in a large ZIM file from the images.

Asynchronous javascript programming is meant to improve responsiveness by mimicking different threads, though in reality it just delays operations that will take a long time until "front-end" operations (visible to the user) have finished. However, what happens when you delay the extraction of stylesheets and images so they all try to happen after the first paint? You get manic to-ing and fro-ing across different parts (front and middle, from "-" namespace to "I" namespace, maybe), all competing with the browser trying to repaint as new CSS and images come in higgledy-piggledy. If xzec.js is being sent backwards and forwards across the file arbitrarily to get stylesheet A, then image Z, then pause to refresh the screen, then stylesheet B, then another repaint, then image Y, then repaint, then stylesheet C then image X, etc., you get a logjam.

Separating out the extraction of stylesheets from the extraction of images and then, possibly, javascript, and not allowing one to proceed till the other is complete, unjams the logs to some extent. Logically, the best file-extraction performance would be achieved by going for one stylesheet at a time, synchronously, and ditching the promise structure for this part of the code (I haven't done that). Even so, xzdec has to jump over a large gap repeatedly to go from extracting the HTML to extracting the stylesheets to extracting the images + javascript. Caching the stylesheets cuts out a huge part of that gap jumping by xzdec, as their retrieval can be left to the browser engine on first paint.

If this makes some kind of sense then the implication is that we should reorganize things like this:

  1. extract HTML synchronously (don't do anything else while the single thread is getting this, it's highest priority) ->

  2. If stylesheets are not cached, extract each stylesheet synchronously, as they are needed for first paint to happen efficiently ->

  3. let loose the image extraction via a promise (asynchronously), because there is no harm waiting for images and they are either all grouped together or grouped with the file, but extract images at the top of the page first ->

  4. Get on with the screen painting as fast as possible, so that image extraction can complete.

@sharun-s 's idea of loading images above the fold first after first paint is excellent. Currently images seem to load from the bottom up, which is useless to humans who tend to read from top down.

I hope this makes some kind of sense and apologies for length.

sharun-s commented 7 years ago

The problem is the asynchronous process

"Uncontrolled" asynchronous task scheduling works well enough in a lot of cases. It's the cases where things break down that require different approaches. One approach is more control of the order. The other approach is more concurrency (i.e. workers). This is a deep rabbit hole but the more people understand it the better the solutions will be. So good to see you working through it.

mossroy commented 7 years ago

@Jaifroid : I've run some tests on your branch on a FirefoxOS device. The images are displaying in commit id 30772b3f9c42a0d16569461a953b19634ca11fd3 , but are not displayed in commit id 1e977fea2df55f5331df1ad45c3b4228aebec5d1, which introduced the regression. After playing with the app in debug mode on the device, I think I found where it comes from.

(a little suspense...)

It "simply" comes from how the src->data-kiwixsrc trick is done. If I remove the string below from the replace regexp, it works : style=\"display: none;\" onload=\"this.style.display='inline'\" In other words, it seems to dislike the hiding/un-hiding images, but changing src to data-kiwixsrc is fine.

Now I should be able to test your CSS changes on my slow and small FirefoxOS devices.

Jaifroid commented 7 years ago

@Mossroy Thank you for testing, and I've now removed the image hiding code. I now think the reason FFOS rejected the onload statement is because there is a missing semicolon:

//Fast-replace img src with data-kiwixsrc and hide image [kiwix-js #272]
        htmlArticle = htmlArticle.replace(/(<img\s+[^>]*\b)src(\s*=)/ig,
            "$1style=\"display: none;\" onload=\"this.style.display='inline'\" data-kiwixsrc$2");

It's the onload=\"this.style.display='inline'\" which should be onload=\"this.style.display='inline';\". However, it's now gone altogether, but may be worth bearing in mind, e.g. for #173 (Display images only on user request), as a variation of this code could be used to avoid rendering gaps where the images should be if they are not brought from the ZIM.

mossroy commented 7 years ago

I now see that you were suspecting it before I found out (https://github.com/kiwix/kiwix-js/commit/17588abe7e37e6f42598d017b9b3662ce13c23b1#commitcomment-23145180). Sorry, I had missed that comment. In any case, the problem is gone on FirefoxOS with your latest commit : thanks

mossroy commented 7 years ago

I did some quick testing on small archives (wikivoyage_fr_all_2015-11.zim, and Ray Charles archive), comparing this branch (with CSS preload) and master, on 2 identical Firefox OS devices (ZIM files in internal memory to have the same access speed) I see differences in the layout, certainly due to different CSS applied (slightly different fonts, some missing borders or colors in this branch). Regarding performance, my first feeling was that it was much faster. I think it comes from the fact that I feel I can start reading the article because there will be no extra CSS applied. It's a clear usability improvement. But, if I compare the time to have the whole page rendered with master, the difference is not big. It's usually a bit faster on this branch.

Sorry I did not have the time to do more tests for now. It's more difficult to test with bigger archives, as I would have to put them on identical big µSD cards (they do not fit in internal memory), which I don't currently have.

mossroy commented 7 years ago

A few more tests : Git commit af10e6bae08e3d8f0f54d8580cfbda60a29c44ab breaks the UI on Firefox OS (but not on Firefox desktop). The main page is not loaded and nothing happens when you use the UI. The log says : SyntaxError: in strict mode code, functions may be declared only at top level or immediately within another function on app.js, for the function sliceImages (and probably serializeImages and loadJavascript) If I remove the strict mode, the UI works again (but it's obviously only a workaround)

Now, if I use the latest commit 92a77b30343ef9efeab4e34a50080d9b819d757a, or af10e6bae08e3d8f0f54d8580cfbda60a29c44ab (and I disable strict mode), the images don't load any more on Firefox OS, the console stops at "Time to First Paint", where on desktop it carries on with "** About to request images # 1 to 10..." etc. I did not investigate why so far. NB : images are loaded with commit d0c63a1f65e4dba98e4384165d73c55c8ce065f3

In all cases, I still see CSS differences (even on desktop) between using cache and not using cache. For example, if I use wikivoyage_fr_all_2015-11.zim with Kiwix 2.1, it looks like this : image It looks the same with this branch with caching disabled. But, with caching enabled, it looks like this : image I suppose a CSS is missing.

Jaifroid commented 7 years ago

Thank you very much, @mossroy . I was actually worried about functions being declared inside functions, but it seemed to work on all the platforms I tested, so didn't try to refactor the code to make sure all functions are declared at the top level or immediately inside another function (thought I'm not sure what "immediately" means in this context). I guess FFOS only supports a certain level of nesting or recursively nested functions in strict mode. I'll see if there's an obvious fix -- moving nested functions up a level, for example.

Can I test FFOS using https://addons.mozilla.org/en-US/firefox/addon/firefox-os-simulator/ ? This is for FFOS 1.1. In the description it says "To test apps with Firefox OS 1.2+, please use WebIDE built into Firefox!" Do you know if the Web IDE in Firefox gives an accurate simulation of FFOS on a phone?

Regarding stylesheet changes, I'll test against the version of wikivoyage fr that is currently on kiwix, though it is a later version (2017-03), and it looks as if wikivoyage changed quite substantially its stylesheets, just from looking at the screenshot you've posted. However, the code is supposed to fall back to getting the stylesheet from the ZIM if it can't find it, so it's probably the case that back in 2015 they were using the same stylesheet name for desktop (style.css) but with different selectors and css content. Will let you know if the issue persists with the latest ZIM.

sharun-s commented 7 years ago

Briefly tested this change. Looks good. I did notice some random articles having weird styles though Denmark - infobox was getting screwed up. China - there is a history timeline that goes off the page.

mossroy commented 7 years ago

@Jaifroid , you indeed can use the Firefox OS simulator. The simulator uses the same web engine (and version) as on a real device. For example, I can reproduce the bugs I mentioned above in the Simulator. I can't say Firefox OS is a main target of course, so don't spend too much time on it. On the other hand, it might show you how searching and choosing the ZIM file works on this platform, and if the app works on older versions of the Firefox web engine (gecko). I'd recommend to use version 2.2 of the Simulator (2.5 is the last released version, but is not available as a simulator, 2.6 was never released). To do that, you simply have to start WebIDE from Firefox (in Development menu), download a simulator version from the GUI, start it by clicking on it, click on "open a packaged application", and point to the main folder of the source code of kiwix-js, then click on the "Play" button at the top of the window. That should launch Kiwix inside the simulator. It should ask you to put some ZIM files on your device, which you can simulate by creating a "fake-sdcard" folder inside extensions/fxos_2_2_simulator@mozilla.org/profile/storage/permanent of your Firefox profile (see https://support.mozilla.org/en-US/kb/profiles-where-firefox-stores-user-data to find it), and put your ZIM files in it (within any folder structure, if you want). NB : for versions <2.2, this folder must be put in extensions/fxos_2_1_simulator@mozilla.org/profile.

Note that you can debug the app too, with the same tools as in Firefox desktop. Also note that, on next restart of Firefox, it will tell you that the Simulator has been disabled (but it will still work)

Jaifroid commented 7 years ago

@mossroy I think I've squashed the FFOS "strict mode" bug with commit https://github.com/kiwix/kiwix-js/commit/bd31cd7f235fc01091e6d756ad511109023d33b2 .

The problem was the if (contentInjectionMode === 'jquery') block in injectHTML() which encompassed some function declarations. I think the latest browsers use the latest ECSMA spec which allows conditional function declarations, whereas FFOS browser is using the old ECSMA spec which doesn't. This is a guess. In any case, I've moved the image functions out of the scope of that conditional, by calling them from within the if condition but not declaring them within the if condition. FFOS in the Firefox Simulator now runs this commit in strict mode. (And thanks for the guidance on setting up FFOS Simulator in Firefox -- worked like a charm.)

If you try it, you will notice images are still squashed, but they now show and the check box for turning them off now works. I will work on unsquashing the images (I have an idea for an easy fix), but I wanted to let you know that the branch now appears to be working in FFOS. Will post again when image aspect ratio is cured, so you may want to wait till then.

mossroy commented 7 years ago

I confirm, it now works out-of-the-box with Firefox OS. Thanks for the fix, it probably fixes this issues for some other platforms too.

mossroy commented 7 years ago

A quick test with one device on master and another one on this branch shows a huge usability difference between them. With your optimizations, it's much quicker (on big articles) to see the beginning of the article (with its images), and be able to start reading it. I still see CSS differences, though (testing on wikivoyage)

Jaifroid commented 7 years ago

That's good to know, thanks @mossroy . I haven't worked on CSS since you last tested -- that's next on my list. Will let you know when I've tested Wikivoyage France.

Note that I've just committed a correction to the mime type declaration for one-by-one images (I had stupidly left it as "text/css" as a result of re-using stylesheets code). It doesn't make any visual difference, surprisingly.

TODO: Fix image maps in Wikivoyage (showing as elongated); fix stylesheet transformations for Wikivoyage ZIM files; fix radio button selection on FFOS for switching between desktop / mobile CSS (currently stuck on desktop and can't be changed, FFOS issue only).

Jaifroid commented 6 years ago

@Jaifroid , you indeed can use the Firefox OS simulator. The simulator uses the same web engine (and version) as on a real device. For example, I can reproduce the bugs I mentioned above in the Simulator. I can't say Firefox OS is a main target of course, so don't spend too much time on it. On the other hand, it might show you how searching and choosing the ZIM file works on this platform, and if the app works on older versions of the Firefox web engine (gecko). I'd recommend to use version 2.2 of the Simulator (2.5 is the last released version, but is not available as a simulator, 2.6 was never released). To do that, you simply have to start WebIDE from Firefox (in Development menu), download a simulator version from the GUI, start it by clicking on it, click on "open a packaged application", and point to the main folder of the source code of kiwix-js, then click on the "Play" button at the top of the window. That should launch Kiwix inside the simulator. It should ask you to put some ZIM files on your device, which you can simulate by creating a "fake-sdcard" folder inside extensions/fxos_2_2_simulator@mozilla.org/profile/storage/permanent of your Firefox profile (see https://support.mozilla.org/en-US/kb/profiles-where-firefox-stores-user-data to find it), and put your ZIM files in it (within any folder structure, if you want). NB : for versions <2.2, this folder must be put in extensions/fxos_2_1_simulator@mozilla.org/profile.

@mossroy In the summer you gave me the instructions above (on this thread) and they worked like a charm. However, recent versions of Friefox seem to have disabled the FFOS Simulator. I have managed to re-enable the Legacy addon / extension (Firefox OS 2.2 Simulator), but it no longer shows up in the Web IDE, so no way to run the app. I'm using Firefox Developer Edition (because legacy addons cannot be enabled in the standard Firefox). Do you know how to get it working again? It was very useful for ensuring for debugging and flagging anything non-standard.

mossroy commented 6 years ago

It still works with Firefox 52 ESR (which still accepts legacy extensions, and will be supported until june 2018). You can get it from https://www.mozilla.org/en-US/firefox/organizations/

Jaifroid commented 6 years ago

Brilliant, thank you!

Jaifroid commented 1 year ago

Image loading performance has been thoroughly addressed.