Page MenuHomePhabricator

Find out what it would take for VisualEditor loading progress bar to be correct
Closed, ResolvedPublic

Description

Find out what it would take for VisualEditor loading progress bar to be correct. Can we get the loading progress from the browser and put it into the progress bar?

Event Timeline

matmarex created this task.Feb 13 2019, 1:16 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 13 2019, 1:16 PM

Change 490414 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] [DO NOT MERGE] Prototype for precise loading progress bar

https://gerrit.wikimedia.org/r/490414

Change 490415 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] [DO NOT MERGE] Prototype for precise loading progress bar

https://gerrit.wikimedia.org/r/490415

Change 490414 abandoned by Bartosz Dziewoński:
[DO NOT MERGE] Prototype for precise loading progress bar

Reason:
(See task for an explanation)

https://gerrit.wikimedia.org/r/490414

Change 490415 abandoned by Bartosz Dziewoński:
[DO NOT MERGE] Prototype for precise loading progress bar

Reason:
(See task for an explanation)

https://gerrit.wikimedia.org/r/490415

I spent a few hours finding out. The conclusion: it could be done, but it would take a lot of work, we would need support from folks in Performance and SRE, and from whoever owns RESTBase these days, and the end result would probably still feel worse to users than the current fake progress bar.


I started out knowing that this is possible, because a few years ago I worked on the upload dialog, where we implemented a precise progress bar (albeit for the upload, rather than the download). The relevant code lives in resources/src/mediawiki.api/upload.js, using the XHR 'progress' event and the notify()/progress() feature of jQuery promises.

Problems to solve:

  • ResourceLoader module loading does not use XHR; instead it uses dynamic <script> elements (see addScript() in mediawiki.js). That code bears dire warnings about not using XHR (because of problems with caching and debugging), but they were written in 2011 or so and may not actually be correct today. We'd need to carefully test various browsers to figure out if we can just ignore this and maybe ask Performance folks to help.
    • Changing that code to use XHR is a bit tricky, because code in mediawiki.js must not rely on jQuery, so we can't even use $.ajax or $.Deferred. We would need to write this carefully to not require those. (For the prototype I ignored that and used jQuery anyway.)
    • In the end, progress notifications using notify()/progress() are added on the promise returned by mw.loader.using().
  • Data loading using mw.Api() doesn't have progress notification, but these are easy to add using the same method (done in the prototype). Data loading from RESTBase using $.ajax doesn't have progress notification, but this again should be easy to add (I did not try to do this, since my local development setup doesn't have RESTBase right now).
  • There are some requirements for the HTTP responses generated by the server to make the XHR 'progress' event useful:
    • Server response must include a Content-Length header (otherwise we only know how much we transferred, but not how much is remaining, so we can't do a progress bar). In Wikimedia production, RESTBase and load.php responses have it, but api.php responses don't, and I have no idea why (perhaps someone in SRE could figure out why and how to change that). Locally, I couldn't get them to appear at all, until I haphazardly hacked up some code in OutputHandler.php. This would require some work to make it reliable.
    • In Chrome and Edge (and probably also Safari and IE), progress is not calculated correctly for gzip-compressed responses: the amount transferred refers to decompressed data, but Content-Length is the length of compressed data. See https://stackoverflow.com/a/47287125 and https://bugs.chromium.org/p/chromium/issues/detail?id=463622. (Firefox is the only browser handling this correctly.) To get a sensible progress percentage, we would have to calculate the decompressed data length on the server and send it to the browser in another custom header. I implemented this in the prototype, but that only handles MediaWiki; handling RESTBase would require more work.
  • Progress notifications are not automatically "forwarded" when chaining promises using .then(), so we have to awkwardly do foo.progress( bar.notify ) in a bunch of places.

Okay so all of this is unfun, but we could work with it, and I wrote some prototype code that made it happen. And in the end I was rewarded with this disappointment:

Turns out that browsers (pictured here is Firefox, but it's the same in Chrome and Edge) fire the 'progress' events very infrequently. The smallest change between two progress events I ever observed was 32KB (even when throttling the connection using developer tools), which is more than the total amount of data we're going to transfer for most pages… Additionally, there are no progress events until the server starts sending a response to our request (because the browser itself doesn't know what the progress is here), and for most pages waiting for the server in fact takes longer than actually sending/receiving the data.


We could probably make some more effort to fudge, smooth and fake the progress, but it would be even more work to design an algorithm for that, and the amount of engineering needed to get us to this point is such that I feel the entire thing is totally not worth it.

matmarex closed this task as Resolved.Feb 13 2019, 9:25 PM