Sometimes the article is briefly displayed then replaced by a blank page

[mossroy]

I noticed this issue with the current version of Firefox (58.0.2), not with Chromium. Both in jQuery and ServiceWorker mode.
If I look at the source of the white page, I see that it's our dummyArticle.html.
It's probably a regression of #342, as I cannot reproduce this issue with the commit before the merge of this PR.

[mossroy]

It seems to be a race condition between loading the dummyArticle (in order to void the frame) and displaying the actual article.
I managed to fix that with the following code in readArticle function (app.js) :

            //Void the iframe
            document.getElementById("articleContent").src = "dummyArticle.html";
            $("#articleContent").off("load");
            $("#articleContent").on("load", function(){
                selectedArchive.readArticle(dirEntry, displayArticleInForm);
            });

But it might not be the best way to do it.

[mossroy]

I'm surprised that the browser can take so much time to load a very simple HTML file.
It might be one of the recent optimizations of Firefox (since v57), where they give a lower priority for loading some content considered less important (ads, trackers etc).

Instead of waiting for the iframe to be ready to start reading the article, it would be better to start reading it right away (it can take some time). At the end of the backend job, instead of calling displayArticleInForm, we might first check that the iframe is loaded. I did not investigate on how to do that yet.

[Jaifroid]

Hmm. When I was initially experimenting, I couldn't put the voiding code in displayArticleInForm, which is where I thought it should naturally go, because it emerged that whatever method I used to load the iframe.src (even just blanking it) was actually happening async. That's why I had to put it in readArticle, since the callback there gives time for the article to load.

Maybe we need to wrap it in a promise or something. Either a simple setTimeOut promise or using q?

[Jaifroid]

I haven't seen this issue in Edge or Firefox ESR (52.6), after many hours of testing in Kiwix-JS-Windows, so it must indeed be something in the latest FF.

[mossroy]

We must avoid a setTimeout, because it would just hide the race-condition issue, that could still occur on slow devices.

I would suggest to move the code that voids the frame to the moment we really need it. It has the advantage to be more readable, and more user-friendly in case of an unexpected error (for example, if something goes wrong in the backend, the current code leaves the user with an empty page. If we move the code in displayArticleInForm, it would leave the existing content).
Maybe you already tried that, but there might be another technical way to void the iframe, that could be tested : using jQuery .empty() on the iframe : https://api.jquery.com/empty/ . I don't know if it really removes the references you need. But, if it does, it seems to be synchronous (no need to wait for a "load" event).
If .empty() is not suitable, I would simply suggest to move the code in displayArticleInForm, use the dummyArticle, and wait for the "load" event to do the rest of the function.
As this dummyArticle is a static page, I suppose the browser will put its DOM in cache, so it might be fast enough.

[Jaifroid]

I was about to post that the code below is a simpler version of your solution above and works in Edge, Firefox ESR and FFOS with no noticeable slowdown:

//Void the iframe
            var iframe = document.getElementById("articleContent");
            iframe.src = "dummyArticle.html";
            iframe.onload = selectedArchive.readArticle(dirEntry, displayArticleInForm);

But I'll look into your ideas above.

[Jaifroid]

First up, as per https://stackoverflow.com/a/342991, we need to use onreadystatechange rather than onload because the latter fires before the iframe document is fully loaded. So, we could simply change the event in the code above to iframe.onreadystatechange and reverse the order (set the event before setting the iframe.src, though I'm not sure it really makes a difference).

Cosmetically, we can move this code into a new function called voidIframe. Commit f560c1b shows what this looks like. It works on Firefox ESR, Edge, Internet Explorer, FFOS.

I have yet to experiment with jQuery.empty() .

[Jaifroid]

I've now tested it, and running $('#articleContent').empty(); and $('#articleContent').contents().empty(); do not clear the scripts from the contentDocument DOM. Moreover, running $('#articleContent').empty(); prevents links from working in Firefox, because of our code that checks links to see whether they use the same protocol as the outer document: Firefox sees the empty .src element and assumes that the protocol isn't the file:/// protocol. Edge doesn't have this issue.

As I mentioned before, if we want to get rid of dummyArticle, we are going to have to revisit that code. We would then be able to use innerDocument.open(), innerDocument.write(), innerDocument.close() to write the content to the iframe which voids the document and its DOM elements while it is writing. The good news is that having the outer document use the file:/// protocol and the innerDocument have no protocol doesn't seem to cause cross origin issues at least in Firefox and Edge running from the filesystem, nor in app context for UWP.

But I see this as a bigger development than the current code. For now, what seems to work with the latest Firefox Quantum, albeit running in Linux, because I couldn't install it alongside Firefox ESR on Windows, is this code. @mossroy , what do you think of this approach in principle (subject to proper testing in a PR)? Is there anything else I should try first?

[Jaifroid]

I'm afraid the solution in the commit referenced above is not actually working universally. I'm getting weirdness somehow related to #348 , and need to eliminate that. Further experimentation needed here, I'm afraid...

[Jaifroid]

Working with https://github.com/kiwix/kiwix-js/tree/Eliminate-unhandled-exception-in-JavaScript-code, which is a branch of master that has the JavaScript routine commented out, I am unable to reproduce this issue in Firefox 58.02 on Linux (Ubuntu) and with 59.0 on Windows (I decided to install it for testing). I'm also unable to reproduce it on master, however. This is in jQuery mode, as Service Worker mode continues to show the API as available but unregistered and selected it puts the browser in a mode where pictures and links do not work. I am using file:/// protocol.

@mossroy could you give me some steps to reproduce? A specific ZIM?

[mossroy]

OK, too bad for jQuery .empty().
Let's keep the dummyArticle : it's not a big issue.

Regarding onload vs onreadystatechange : I don't understand why onload would not be appropriate here. Plus if you use onreadystatechange, you need to first test if the state is the one you want : this event is fired on each state change (including a change to a state you don't want). In the stackoverflow link you mention, they test for the state "complete". That might be the cause of #348 : if the frame is not fully initialized, its attribute src might not be available at this time.

All in all, I would suggest to keep using the onload event (the difference with the readystate "complete" seems to be only for the loading of images or other dependencies, which we don't have in this very simple HTML file), and to put the event handler after setting the "src" attribute.

Regarding using a voidFrame function : why not, but it should be renamed to something like voidFrameAndDisplayArticle to be more explicit on what it does.

It can be difficult to reproduce the blank page : sometimes you have it on every page, sometimes not at all. I don't think it depends on the ZIM file. I had it at least with askubuntu.com_en_all_2017-06.zim

[mossroy]

Sorry, I think I spoke too soon about the link with #348 : it's probably unrelated as one is about an iframe tag, and the other one about a script tag.

[Jaifroid]

I managed to reproduce this issue in latest Firefox on kiwix-js-windows, when calling the page's html from the standard localStorage API (I've now got some code that stores the html of the current page in localStorage, for fast page transforms). It only happens in the latest Firefox, not in Edge or IE. Effectively, if the html is injected into the DOM before dummyArticle.html is loaded, then dummyArticle subsequently erases it. Firefox is clearly "lazy loading" dummyArticle.

It's a slightly different case on kiwix-js-windows, because normally the injection of html doesn't occur until all CSS is loaded (as a result of my early preload CSS code experiments), and this makes the problem disappear normally due to the callbacks and promises involved in CSS loading. But when fast-retrieving the html from localStorage when the code effectively reloads everything from cache, without promises or callbacks, I finally managed to reproduce this issue albeit in this slightly different form. For that specific case, in kiwix-js-windows, a simple setTimeout delay of 100 milliseconds solves the problem at least for now. For the more general case, we still need to find a way to detect reliably that dummyArticle is fully loaded.

[mossroy]

Good that you managed to reproduce.
IMHO, the onload approach that we discussed above should be a reliable fix.

[mossroy]

Fixed by #354