1643508 - with apz.allow_zooming = true touch screen pinches zoom the whole page including pdf.js controls

Status: RESOLVED FIXED

(Firefox :: PDF Viewer, defect)

Description

[Timothy Nikkel (:tnikkel)]

This is basically bug 1625462 but for touchscreen pinches instead of touchpad.

Comment 1

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:bdahl, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Botond Ballo [:botond]]

Theory: perhaps we are expecting pdf.js to preventDefault() touch events in this scenario, and it's not doing so?

Comment 3

[Kartikaya Gupta (email:[e-mail])]

pdf.js doesn't seem to handle touch events at all outside of "presentation mode". Even in presentation mode it only handles swiping (zooming is not allowed in presentation mode). So we have to implement zooming in pdf.js to make this work.

Comment 4

[Botond Ballo [:botond]]

Did touchscreen pinch gestures have any effect in pdf.js with apz.allow_zooming=false?

Comment 5

[Kartikaya Gupta (email:[e-mail])]

On my Windows laptop it looks like it did reflow zoom of the entire document (including UI bits). Which seems like what I would expect.

Comment 6

[Botond Ballo [:botond]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #5)

On my Windows laptop it looks like it did reflow zoom of the entire document (including UI bits). Which seems like what I would expect.

Interesting. On Linux, reflowing zoom (activated via Ctrl+mousewheel) zooms the PDF content but not the controls.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Yeah that's handled by pdf.js, they preventDefault the ctrl+mousewheel and zoom the content. So it's not actually triggering browser reflow zoom. Touch doesn't dispatch mousewheel though.

Comment 8

[Botond Ballo [:botond]]

Ah, I see.

So we could probably at least restore the previous behaviour (of reflow-zooming the PDF controls) by having APZ take the NotifyPinchGesture codepath for pdf.js pages (but it would require APZ knowing that it's a pdf.js page), but matching the behaviour of ctrl+mousewheel (which is to not zoom the PDF controls at all) would require changes to pdf.js like you said.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

I don't think reflow zoom is a particularly useful thing to do on pdf.js because the UI ends up looking smushed. I intend to just disable all pinch behavior by calling preventDefault on the second touchstart. Eventually it could have it's own pinch zoom implementation but that seems out of scope for this bug and might require UI fiddling in pdf.js as well.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

https://github.com/mozilla/pdf.js/pull/12212

I realized though that with apz.allow_zooming=true we also support the one-touch-pinch gesture, which is kind of an Android-ism that we probably don't want to have on desktop. In particular calling preventDefault() on a multi-touch touchevent doesn't prevent zooming via one-touch-pinch so it's still possible to zoom on things like pdf.js (with above PR) using the gesture. I'll file an APZ bug for that.