1660054 - Can't reset new pinch zoom

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[hecerinc]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:81.0) Gecko/20100101 Firefox/81.0

Steps to reproduce:

On my laptop:

Opened https://www.w3.org/TR/WCAG21/#operable, pinched to zoom with the trackpad.

Actual results:

The new pinch zoom that shipped in FF 80+ (I think it was 81) where the content doesn't reflow but instead just zooms (like Chrome does) was activated and the content was zoomed. But then I connect my laptop to my hub and can no longer use the trackpad to zoom out. However, the pinched zoom persists, and I can't zoom out through normal means. (Ctrl scroll)

Expected results:

I should have a way to reset this zoom like the normal reflow zoom can via the URL bar.

Comment 1

[Hani Yacoub, Desktop QA]

Assigning "Core: Panning and Zooming" component.

Comment 2

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

(In reply to hecerinc from comment #0)

But then I connect my laptop to my hub and can no longer use the trackpad to zoom out.

Can you describe a bit more what hardware you're using? Which laptop and which hub?

Comment 3

[Botond Ballo [:botond]]

As a workaround while the issue is fixed, reloading the page with Ctrl+Shift+R should reset the pinch zoom.

Comment 4

[hecerinc]

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

(In reply to hecerinc from comment #0)

But then I connect my laptop to my hub and can no longer use the trackpad to zoom out.

Can you describe a bit more what hardware you're using? Which laptop and which hub?

I'm using a ThinkPad P52S, and the ThinkPad Hybrid USB-C with USB-A dock, although I suppose the problem would still be there with any other laptop. The basic gist being that if you (for any reason) lose access to the trackpad, there's no GUI control to reset the pinch zoom.

(In reply to Botond Ballo [:botond] from comment #3)

As a workaround while the issue is fixed, reloading the page with Ctrl+Shift+R should reset the pinch zoom.

This did work :) A bit hidden for most users, I think.

Comment 5

[hecerinc]

By the way, for the zoom team that implemented this: Top notch job! 💯 I absolutely love this feature and have been looking forward to FF implementing this for years! Thanks!

Comment 6

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

Martin, any thoughts on this? Seems like a good argument to expose a smooth-zoom indicator in the UI (or at least some way to reset it).

Comment 7

[Martin Balfanz [:mbalfanz]]

Yea, good call. Thanks for bringing this to my attention!

The indicator might be an option. It also reminds me that we were talking about mouse interactions for this feature. Let me think about this a bit and also consult with UX to get some more feedback.

Comment 8

[Martin Balfanz [:mbalfanz]]

(personally, I lean towards the indicator as it will help all users. And as the reporter said, it makes the force reset more discoverable as well. But with the desktop team working a lot on the UI right now I want to make sure we have them on board)

Comment 9

[Timothy Nikkel (:tnikkel)]

I saw in another bug a request to make ctrl-0 also reset the pinch zoom (it currently only resets the reflow zoom), which seems reasonable at first thought.

Comment 10

[Martin Balfanz [:mbalfanz]]

That is an excellent suggestion! +1 from me.

UX was against an additional UI component, and I think this known keyboard shortcut strikes a great balance.

Comment 11

[Martin Balfanz [:mbalfanz]]

It took a while, but there is a clear preference to not add anything to the UI. Resetting zoom using ctrl/cmd-0 is the preferred option.

Comment 12

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

We'll probably want to change the line here to do something like FullZoom.reset(); FullZoom.resetApzZoom() and add the latter function to actually reset APZ zoom. Or we could put the function not in FullZoom, but regardless, we presumably will want it hooked up to cmd_fullZoomReset.

Comment 13

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

I can use this bug to hook up ctrl+0.

Comment 14

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

Attachment: Bug 1660054 - Hook up the zoom-reset action to also reset APZ/scaling zoom. r=Gijs!,nika!,botond!

The goal here is to hook up the ctrl+0 keyboard shortcut to reset the scaling
zoom applied by pinch gestures (on touchscreen or trackpad), in addition to
resetting the reflow zoom (aka full zoom). This patch also makes other mechanisms
to reset the reflow zoom (e.g. clicking on the "100%" label in the hamburger menu)
also reset scaling zoom, which I think makes sense for consistency.

Most of this patch is just plumbing, but I'm unfamiliar with these codepaths
so requesting review from relevant owners to make sure it's sane.

Comment 15

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

Hi Stuart, a question came up about interaction between this bug and the WebExtensions API, and I was hoping you can redirect to somebody who can provide a definitive answer. As you may be aware, in Firefox 83 we are shipping a "pinch zoom" feature that does mobile-style scaling zoom on desktop. This bug is about adding a mechanism for users to "reset" the zoom back to the default initial zoom. Based on feedback from product, the patch above hooks up the ctrl+0 keyboard shortcut (currently used to reset reflow zoom) to also reset the new scaling zoom. Similarly, the "100%" buttons in the hamburger menu and URL bar that reset the reflow zoom will also reset the scaling zoom.

There is apparently also some webextension API surface area that allows extensions to control the zoom. In particular, the tabs.setZoom() API can also trigger a reset of the reflow zoom. Should this API also trigger a reset of scaling zoom? I was leaning towards no, since this is a programmatic use case - existing extensions may be using this API in such a way that if we also make it reset scaling zoom that would produce unexpected behaviour for the user. But on the patch :Gijs brought up the fact that the existing API also affects things like pdf.js and about:reader so maybe it makes to also reset scaling zoom. Do you know who can make a call on this?

Comment 16

[Rob Wu [:robwu]]

(Stuart is out, but I'll answer the question)

I suggest to not reset the pinch zoom level via browser.tabs.setZoom(tabId, 0), because it would be inconsistent to offer a way to reset it without the ability to set it through the API. If there is ever demand for the ability to (re)set pinch zoom via an extension, then we can expand the API to support it (file a new bug in WebExtensions::General ).

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/952da6bff887
Hook up the zoom-reset action to also reset APZ/scaling zoom. r=Gijs,nika,botond

Comment 18

[Alexandru Michis [:malexandru]]

Backed out changeset 952da6bff887 (Bug 1660054) for causing bc failures in browser_test_reset_scaling_zoom.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/3eeddf8bb219d42e11e3978de117675be3673810

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=952da6bff88756cb68e515b88d134e22c831a566&selectedTaskRun=IC7SmF2YQDyku-iSYz6zXA.0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=320059797&repo=autoland&lineNumber=4659

Comment 19

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

Ah, looks like it's failing on Windows because the pinch zoom synthesization is failing. I'll fix and retest before landing.

Comment 20

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=194db6718d130ebec1c05d97e072e37e8ce33856

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1acd12e7660e
Hook up the zoom-reset action to also reset APZ/scaling zoom. r=Gijs,nika,botond

Comment 22

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/1acd12e7660e

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 24

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

Comment on attachment 9183509 [details]
Bug 1660054 - Hook up the zoom-reset action to also reset APZ/scaling zoom. r=Gijs!,nika!,botond!

Beta/Release Uplift Approval Request

• User impact if declined: User may find it harder to reset the pinch zoom after zooming in.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Pinch zoom in on various pages and verify that the following methods all work to reset the pinch zoom:

• clicking the "100%" label in the hamburger menu
• pressing the ctrl+0 keyboard shortcut
• changing the reflow zoom (e.g. via ctrl++ or ctrl+-) and then clicking the zoom percentage indicator that appears in the url bar

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Most of the patch is just plumbing, the actual change is pretty small. It's not an urgent thing to uplift as product was ok shipping the pinch zoom feature without this, but since the patch is done and relatively straightforward I figured I'd request uplift anyway.
• String changes made/needed: none

Comment 25

[Pascal Chevrel:pascalc]

Comment on attachment 9183509 [details]
Bug 1660054 - Hook up the zoom-reset action to also reset APZ/scaling zoom. r=Gijs!,nika!,botond!

It's a P3/S3, we have shipped several releases with this bug and I am not seeing duplicate reports of this bug which would help justify the uplift, this can probably just ride the trains, thanks though!

Comment 26

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

(In reply to Pascal Chevrel:pascalc from comment #25)

Comment on attachment 9183509 [details]
Bug 1660054 - Hook up the zoom-reset action to also reset APZ/scaling zoom. r=Gijs!,nika!,botond!

It's a P3/S3, we have shipped several releases with this bug and I am not seeing duplicate reports of this bug which would help justify the uplift, this can probably just ride the trains, thanks though!

Just to be clear, this isn't a bug that we have shipped on release. It's only a bug when desktop/pinch zooming is enabled, which is happening in 83 release. You could consider this a completeness bug for the new feature in 83.

Comment 27

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox Nightly 84.0a1 (2020-10-30) on Windows 10 x64 (Touchscreen and touchpad) and on MacOS 10.15 (Touchpad).

Comment 28

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

Well I guess it's too late to uplift now.

Comment 29

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

Actually I'll put the needinfo back for now, would be good to get a confirmation from Pascal that he really doesn't want to uplift this despite the misunderstanding above. Uplifting this would help the user experience with the new pinch-zooming feature, as we have had a few people complain about accidentally triggering it and not knowing how to reset it.

Comment 30

[Martin Balfanz [:mbalfanz]]

I'd like to second that. Resetting has been the number one problem that was reported during the Nightly/Beta testing. This patch addresses that and enhances the first impression a lot.

Comment 31

[Pascal Chevrel:pascalc]

Comment on attachment 9183509 [details]
Bug 1660054 - Hook up the zoom-reset action to also reset APZ/scaling zoom. r=Gijs!,nika!,botond!

Hi, given the additional context information, I am taking this patch in beta, thanks!

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/da6a77d55dbd

Comment 33

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 83.0b9 on Windows 10 x64 (Touchscreen and touchpad) and on MacOS 10.15 (Touchpad).