[api-minor] Respect the drawingDelay also when CSS-only zooming is used (issue 18022)

[Snuffleupagus]

If a user manually calls PDFPageView.prototype.update() with a drawingDelay-option then it'll always be necessary to re-call the method without a delay afterwards, regardless of the maxCanvasPixels-value (e.g. even when CSS-only zooming is used).

[Snuffleupagus]

/botio-linux preview

[moz-tools-bot]

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/a24ee192fedf149/output.txt

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Success

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

Total script time: 1.91 mins

Published

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

[Snuffleupagus]

/botio integrationtest

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Received

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

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

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 1

Live output at: http://54.193.163.58:8877/266bf72cb5fe464/output.txt

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Success

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

Total script time: 7.13 mins

• Integration Tests: Passed

[moz-tools-bot]

From: Bot.io (Windows)

---

Failed

Full output at http://54.193.163.58:8877/266bf72cb5fe464/output.txt

Total script time: 19.54 mins

• Integration Tests: FAILED

[timvandermeij]

The change looks good to me aside from the comment above, and two questions:

• I assume it's not really easy/possible to add a test for this particular issue, but it might be good to add a more general test that zooms in to above the CSS zooming threshold and asserts that the rendering still works so we have at least hit the CSS zooming feature in the integration tests. However, that doesn't have to be part of this PR per se since it's not directly related to the issue at hand, so I'll leave that up to you to decide.

• Should this be tagged as [api-minor] since I thought we mainly used that for the library API and not really for the viewer API (since I don't think we really defined a formal viewer/components API)? It is a possible behavior change for viewer consumers, so I'm not against tagging it, but figured I should verify since I'm not really sure what the scope of that tag is.

[nicolo-ribaudo]

I assume it's not really easy/possible to add a test for this particular issue

I believe it is possible to test this by listening to the pagerendered event while zooming, I can try providing a test in the next few days.

[Snuffleupagus]

The change looks good to me aside from the comment above, and two questions:

• I probably won't have time to create tests for this, unfortunately.

• Given that this is part of the viewer components, note https://github.com/mozilla/pdf.js/blob/master/web/pdf_viewer.component.js which "power" these examples, I figured that [api-minor] might not hurt. Although you're obviously correct that this isn't the primary library API.

[timvandermeij]

I believe it is possible to test this by listening to the pagerendered event while zooming, I can try providing a test in the next few days.

If that's possible that'd be great; thanks! It's not a must-have for landing this PR, but if it's possible to have some (basic) testing for either this issue or CSS zooming in general that would increase confidence in the code.

[...] I figured that [api-minor] might not hurt [...]

I agree. Let's keep this as-is then.

[timvandermeij]

The change looks good from my point of view, now that the questions above have been addressed. I'd prefer we wait a few days with merging this until we know if we can include a test for this, but otherwise I'm also not opposed to doing that in a separate PR if that turns out to be more complicated than anticipated.

[nicolo-ribaudo]

@timvandermeij Unfortunately I don't think I'll be able to submit a test until next week.

[timvandermeij]

@nicolo-ribaudo No worries; we're already happy that you're willing to help out with this, so no rush. Let's land this PR so the issue is resolved, and I have opened follow-up issue #18105 for introducing tests for this functionality (since that scope is a bit broader than just this particular patch).

@Snuffleupagus Thank you for the patch!