Display the content immediately after a search in ServiceWorker mode.

[mossroy]

Instead of waiting for the "load" event.
On big articles, it allows some browsers to display the content even if it is not fully loaded (missing images etc).
It can make a big difference in the user perception of performance.
Fixes #413

[Jaifroid]

Great, I'll test this PR shortly on Edge etc.!

[Jaifroid]

This PR will cause a regression to #381 (serious performance regression on rendering the index page of Stackoverflow.com) in the Edge browser in Service Worker mode only. I believe it won't cause that regression in Firefox or Chromium, because they appear to wait for the stylesheets to be ready before rendering the page. Edge, however, does not wait, and starts rendering whatever the Service Worker receives as soon as it receives it. With this PR I see (visually) multiple page redraws as the CSS is applied in Edge, and CSS is applied after some images are already downloaded. This means that at least for Edge we would need to prioritize sending CSS to the Service Worker, and ideally not render until CSS has been sent.

On the other hand, this PR provides a clear user optimization for Firefox and Chrome, but a regression for Edge in Service Worker mode only with a minor use case. (It does slightly spoil the overall experience in Edge in SW mode, because the page is not really readable while CSS is being applied -- it jumps around.)

Thoughts? Swings and roundabouts!

[Jaifroid]

The branch explained in #412 (https://github.com/kiwix/kiwix-js/tree/Service-Worker-queue) builds on your PR, but modifies it somewhat so that the page is rendered (with $('#articleContent').show();) straight after the last CSS is sent to Service Worker. This solves the issue with Edge, and makes no difference to Chrome or FF.

I've also added $("#articleName").html(title); which I think has been accidentally omitted from this PR? It sets the "Reading article …." message correctly by inserting the correct title.

I didn't want to commit this code to your PR, because it's done as a test, and I also silenced a lot of the logging done in Service Worker so that I could focus on what was being sent back to SW. If you like the approach, I could commit the functional core of the code to this PR.

I think the branch could be the beginning of a queue for Service Worker, which is why I've named it Service Worker Queue. However, it currently doesn't do any queueing. We can decide in #412 whether to develop the code into a functional queue.

[mossroy]

Interesting. Thanks a lot for catching up so quickly with what I'm doing at the hackathon. It's very valuable to have your feedback and your help.

Regarding Edge in SW mode, does this PR give a significant performance regression? Or "only" a visual annoyance?

[Jaifroid]

Regarding Edge in SW mode, does this PR give a significant performance regression? Or "only" a visual annoyance?

It's the same regression we had with #381, i.e. the little boxes on the Stackoverflow ZIM are redrawn each time a new CSS file is sent to SW, so the browser spends all its time trying to redraw the gazillions of little boxes and the page takes forever to load. It's the reason we prevented render until all CSS was loaded in the first place, to solve #381. The solution via PR #387 (Prevent render until CSS fulfilled) was to count the CSS and render when all of them were ready, but that fix was only in jQuery mode, because SW wasn't affected (until this PR).

It seems we have four possibilities:

1. Not apply this PR;
2. Deem Edge on Service Worker to be an "edge" case (pun intended!) and apply this PR;
3. Apply a version of Prevent render until all CSS is fulfilled #387 to Service Worker, i.e. the Service Worker queue branch, or
4. Apply a more comprehensive solution such as the Universal queue.

Your call! They're not necessarily mutually exclusive options (except 1, perhaps). Others could be seen as interim stages towards a wider solution.

[mossroy]

As discussed in #412, I believe adding queuing in SW mode breaks the separation of responsibilities of the layers, and might prevent us from doing refactorings like using the libzim as the backend.
So that would leave us with options 1 or 2.

[Jaifroid]

Probably better to go for 2, as it provides the greater good to the greater number of users, and users of Edge are much more likely to be using the app in jQuery mode. However the PR doesn't display the spinner and "Reading article xxxxxx from archive. Please wait...." when clicking on a link in an article. It only does it when searching for a file. Can you add that? It seems inconsistent as it currently is. This would also address #155.

[mossroy]

I tend to agree.
But some time we'll certainly implement #196 to switch to SW mode by default. hopefully the Edge engine will be improved in between?

Regarding the spinner, I agree that it's not consistent and should be improved.
But, instead of trying to add the spinner everywhere, I would suggest to not display it after a search (only in SW mode). Let me explain :
In SW mode, the browser makes use of its own way to inform the user that a page is loading. The way it is done depends on the browser brand/version. See https://www.stevesouders.com/blog/2013/06/16/browser-busy-indicators/
I think we should simply rely on that, without trying to add our own way to inform the user.

Plus it would be technically complicated. I only see a few bad solutions :

• modify the DOM from handleMessageChannelMessage, which looks wrong to me
• parse and modify the HTML string in the ServiceWorker, to inject some javascript code that handles this
• parse and modify the DOM in the load event of the iframe to inject some javascript code that handles this

[Jaifroid]

I think this bug (?) in Edge would be a blocker for moving the entire app to SW mode, as the experience is then very unprofessional. Given, however, that there are pragmatic solutions we would be forced to adopt in that case, and given that Service Worker is a marginal use case that is currently warned against in the app, I've got no objection to your proceeding with option 2.

Personally, I would go with a pragmatic solution to #155 (Adding a spinner), because it's just two lines of code. But I completely respect (and learn from, and also greatly benefit from) your more thorough and pure approach! So I leave that decision to you! :-)

[mossroy]

Thanks @Jaifroid : I've merged this PR, and created #419 for the spinner inconsistency in SW mode.