[api-minor] Use a CSS transform to update the progress bar instead of changing the width (bug 1768481)

[calixteman]

• it isn't a fix for bug 1768481 but just a tiny improvement to refresh the progress bar on the compositor thread.

[Snuffleupagus]

First of all, given that there appeared to be a suggestion in the bug about never using the viewer-loadingBar and instead only use the existing browser loading-indicator (in the tab); is this something that's being seriously considered here?

Because if we end up doing that, which based on looking through the relevant code just now might not actually be that difficult to implement, these sort of changes feels mostly moot to me.

---

Secondly, please keep in mind that the ProgressBar is being exposed in the "viewer components" and that we even have an example using it. That example is now broken, at least the loadingBar-related code, by these changes and so is any other third-party use-case as well.
Please also note how the ProgressBar-class currently accept various options, which this patch silently ignores/overwrites:

pdf.js/web/ui_utils.js

Lines 686 to 703 in 38c8235

	class ProgressBar {
	constructor(id, { height, width, units } = {}) {
	this.visible = true;
	// Fetch the sub-elements for later.
	this.div = document.querySelector(id + " .progress");
	// Get the loading bar element, so it can be resized to fit the viewer.
	this.bar = this.div.parentNode;
	// Get options, with sensible defaults.
	this.height = height || 100;
	this.width = width || 100;
	this.units = units || "%";
	// Initialize heights.
	this.div.style.height = this.height + this.units;
	this.percent = 0;
	}

[marco-c]

First of all, given that there appeared to be a suggestion in the bug about never using the viewer-loadingBar and instead only use the existing browser loading-indicator (in the tab); is this something that's being seriously considered here?

Because if we end up doing that, which based on looking through the relevant code just now might not actually be that difficult to implement, these sort of changes feels mostly moot to me.

I think we should ask an opinion to some UX expert.

[marco-c]

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1769023 to take into consideration removing the progress bar or reducing the cases where it is shown.
Moving the animation to the compositor thread would be good for perf and power usage.

[Snuffleupagus]

Given the second point mentioned in #14898 (comment), if we need to change the CSS used to update the loadingBar it'd probably make sense to explicitly remove support for any options in the ProgressBar-constructor first (since that'd probably simplify things).

[calixteman]

Thanks to this change the animation can run on the compositor thread:

[Snuffleupagus]

Did you forget to test the indeterminate ProgressBar, since that one no longer works :-P

(When I want to manually test that one, I usually change this line to const percent = NaN; instead.)

To fix that, it seems that just replacing

pdf.js/web/viewer.css

Line 382 in 63441e8

width: 100%;

with transform: none; is sufficient.

Finally, please remember to make the same exact changes in https://github.com/mozilla/pdf.js/tree/master/examples/mobile-viewer as well.

[calixteman]

I didn't know about indeterminate state, thank you for pointing that out.
I slightly changed the animation to have it on the compositor thread:

[Snuffleupagus]

/botio-linux preview

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/fb4ff91178a49e2/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.241.84.105:8877/fb4ff91178a49e2/output.txt

Total script time: 2.59 mins

Published

• Viewer: http://54.241.84.105:8877/fb4ff91178a49e2/web/viewer.html
• Viewer (legacy): http://54.241.84.105:8877/fb4ff91178a49e2/legacy/web/viewer.html

[Snuffleupagus]

I didn't know about indeterminate state, thank you for pointing that out.

Note that in practice it's pretty rare to see that one, since it only happens if a server doesn't provide a (valid) contentLength which means that it's impossible to tell how much of the PDF document is loaded; see e.g.

• https://searchfox.org/mozilla-central/rev/6d396b2abda4371f54251e9de8dc790deab706fc/toolkit/components/pdfjs/content/PdfStreamConverter.jsm#197
• https://searchfox.org/mozilla-central/rev/6d396b2abda4371f54251e9de8dc790deab706fc/toolkit/components/pdfjs/content/PdfStreamConverter.jsm#208-211