Closed Bug 1738696 Opened 3 years ago Closed 3 years ago

[Bug]: Always zooms in on form input field when focused

Categories

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

Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
97 Branch
Tracking Status
firefox-esr91 --- wontfix
firefox94 --- wontfix
firefox95 --- wontfix
firefox96 --- fixed
firefox97 --- fixed

People

(Reporter: petru, Assigned: tnikkel, Mentored)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [lang=c++])

Attachments

(2 files)

From github: https://github.com/mozilla-mobile/fenix/issues/22235.

Steps to reproduce

  1. Go to a website that uses an HTML form with an input tag
  2. Tag the input field

Expected behaviour

The browser does not zoom by default. Or at least doesn't zoom when zooming is disabled in some way.

Zooming can be disabled with the viewport meta tag, but also with CSS like touch-action:. manipulation. But this has no effect on the form zooming Firefox Mobile does.

Actual behaviour

The browser zooms in to focus on the input field.

There does not seem to be any way to disable this through CSS or meta tags. There are things that work for iOS, but nothing works for Firefox Mobile. Chrome on Android does not have this problem.

Device name

Samsung Galaxy A50

Android version

Android 11

Firefox release type

Firefox

Firefox version

93.2.0

Device logs

No response

Additional information

See this SO question (not by me) for some more detailed examples of things that work for other browsers, but just don't for Firefox Mobile.

Change performed by the Move to Bugzilla add-on.

  1. Tag the input field

Just to clarify, this was a typo when I originally filed the bug. I meant to write "tap", not "tag".

Component: General → Panning and Zooming
Product: GeckoView → Core

There is a tradeoff here between user control and developer control. From the user's point of view, the zoom-to-focused-input behaviour is in part an accessibility feature to allow users to work with input fields whose contents may be too small to comfortably read at the default zoom level.

That said, we do already have an "Always enable zoom" option in settings, which allows a user to override user-scalable=no on websites. So, we could make zoom-to-focused-input obey user-scalable=no, and users would still have that override available.

I'll bring this up for discussion at our next team meeting.

In the meantime, I would also be interested to hear your motivations for disabling the behaviour. Could you give an example of a website where you'd want to use this, and describe why the current behaviour is a poor experience? There may be an opportunity for us to improve the default behaviour of zoom-to-focused-input when enabled, in addition to providing a way to disable it.

Flags: needinfo?(makeworld)

Thanks for your response.

I definitely get the accessibility concerns, and I think that being able to zoom is important, even if zooming is "disabled" on websites. But that zooming option can still be available even if the zoom doesn't happen by default.

The reason I created this issue is for my website NearTalk which is a toy chat application that uses the input element. When trying to write a chat message on Firefox Mobile, the zooming means that you can't see your message as you're typing, and you can't click the send button. You have to click the input field, type blindly, then hit the Android back button to remove the keyboard, then zoom out and hit send or edit your message. All in all, I think this makes for a poor user experience for the site.

Flags: needinfo?(makeworld)
Mentor: botond
Whiteboard: [lang=c++]

We discussed this and are supportive of making both user-scalable=no and touch-action: manipulation disable zoom-to-focused-input behaviour.

As a relatively straight-forward change, this is a good candidate for someone looking to learn the APZ code, so I marked it as a mentored bug for now. If it doesn't get traction in the next couple of months we'll pick it up ourselves.

Severity: -- → S3
Priority: -- → P3

Thanks, good to hear it. There are potentially other settings that should disable this, like max-scale=1 and touch-action: none.

(In reply to makeworld from comment #5)

There are potentially other settings that should disable this, like max-scale=1

Based on my reading of the code, min-scale and max-scale should already be respected for zoom-to-focused-input. You should be able to use these as a workaround while waiting for the change here to also respect user-scalable.

and touch-action: none.

Definitely. manipulation is the most permissive value of touch-action (other than auto), so if manipulation disables an action, then all the less permissive values (including none) will disable it as well.

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

(In reply to makeworld from comment #5)

There are potentially other settings that should disable this, like max-scale=1

Based on my reading of the code, min-scale and max-scale should already be respected for zoom-to-focused-input. You should be able to use these as a workaround while waiting for the change here to also respect user-scalable.

The site I linked earlier has the following meta tag:

<meta name="viewport" content="width=device-width, height=device-height, initial-scale=1.0, minimum-scale=1, maximum-scale=1, user-scalable=no" />

The zooming issue I originally described still occurs, despite these scale values being set. So I haven't been able to find any workaround for this issue.

(In reply to makeworld from comment #7)

The site I linked earlier has the following meta tag: [...]

The zooming issue I originally described still occurs, despite these scale values being set.

Hmm, I don't actually get zooming on this website when I tap on the input field.

Could you confirm please:

  • What Firefox for Android version you are testing with
  • What device you are using
  • Any particular steps needed to trigger the zooming, or is it just load the page and tap on the input?

Firefox 94.1.2 (Build #2015844587)
Samsung Galaxy A50 (Model number: SM-A505W)
Android 11

is it just load the page and tap on the input?

Yep, that's all that triggers it for me.

Thanks. Turns out I was testing an older version; I do get the zoom with 94.1.2. It appears the behaviour has regressed on our side.

Regressed by: 1713586
Has Regression Range: --- → yes

Set release status flags based on info from the regressing bug 1713586

In AsyncPanZoomController::ZoomToRect we calculate a localMinZoom

https://searchfox.org/mozilla-central/rev/bc5e79f3ae0f42cb4a6ebd05fc32f48a3829059d/gfx/layers/apz/src/AsyncPanZoomController.cpp#5490

as the maximum of the min zoom from the zoom constaints and the ratio of compositions bounds to css page rect. This can lead to localMinZoom being greater than the min zoom on the zoom constraints, and also greater than the current zoom. Then later we clamp and then can thus change the zoom value when the zoom constraints don't allow it.

https://hg.mozilla.org/integration/autoland/rev/32e3e98ca8474ded656e7d6eb877fa1919e5355b moved the clamping to happen after the code checks for ONLY_ZOOM_TO_DEFAULT_SCALE and potentially changes the target zoom value, and this caused this regression.

We do a similar computation when computing the resolution for the first paint (via MobileViewportManager::UpdateResolutionForFirstPaint) we call MobileViewportManager::ComputeIntrinsicScale

https://searchfox.org/mozilla-central/rev/55a826a9ef74e92988e56cd9615d4fc6a470695e/layout/base/MobileViewportManager.cpp#99

which computes the ratio between scrollable rect and composition size. The difference is that MobileViewportManager::ComputeIntrinsicScale subjects this ratio to the zoom constraints, whereas AsyncPanZoomController::ZoomToRect can override the zoom constraints with this ratio. I think the fix is to make AsyncPanZoomController::ZoomToRect match MobileViewportManager::ComputeIntrinsicScale but I'm reading the code history to make sure we don't regress something.

(In reply to Timothy Nikkel (:tnikkel) from comment #13)

In AsyncPanZoomController::ZoomToRect we calculate a localMinZoom

https://searchfox.org/mozilla-central/rev/bc5e79f3ae0f42cb4a6ebd05fc32f48a3829059d/gfx/layers/apz/src/AsyncPanZoomController.cpp#5490

as the maximum of the min zoom from the zoom constaints and the ratio of compositions bounds to css page rect. This can lead to localMinZoom being greater than the min zoom on the zoom constraints, and also greater than the current zoom.

If the min zoom from the zoom constraints is 1, as in this testcase, and localMinZoom is larger than that, that would seem to suggest that the scrollable rect is shorter than (or less wide than) the composition bounds. Should that happen (particularly on a page with width=device-width, i.e. where the initial containing block is sized to the composition bounds)?

Maybe the dynamic toolbar is throwing a wrench into the works by changing the aspect ratio of the composition bounds but not the initial containing block?

Yeah, it does seem as though the composition bounds includes the dynamic tool bar but the scrollable rect does not include the dynamic toolbar.

The scrollable rect doesn't seem to include the dynamic toolbar, whereas the composition bounds seem to, so we can compute a value that is outside of the zoom constraints.

This code was introduced in
https://hg.mozilla.org/mozilla-central/rev/5e0366a64dfc

Assignee: nobody → tnikkel
Status: NEW → ASSIGNED
Attached file Bug 1738696. Add test. r?botond (deleted) —
Pushed by tnikkel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/df27b760a48e
Don't allow AsyncPanZoomController::ZoomToRect to change zoom to a value outside of zoom constraints. r=botond
https://hg.mozilla.org/integration/autoland/rev/c9180ad3534a
Add test. r=botond
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 97 Branch

The patch landed in nightly and beta is affected.
:tnikkel, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(tnikkel)

Comment on attachment 9252098 [details]
Bug 1738696. Don't allow AsyncPanZoomController::ZoomToRect to change zoom to a value outside of zoom constraints. r?botond

Beta/Release Uplift Approval Request

  • User impact if declined: on android, zooming to focused input can zoom on pages that have disabled zooming
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): just clamping the zoom value to predefined min/max, like it should have been all along
  • String changes made/needed:
Flags: needinfo?(tnikkel)
Attachment #9252098 - Flags: approval-mozilla-beta?
Attachment #9252486 - Flags: approval-mozilla-beta?

My understanding from reading this issue is that setting minimum-scale=1 and maximum-scale=1 alone will fix this bug in Firefox 97 and later. Is that correct? What about user-scalable=no? Does setting that alone work?

From what I've read, setting touch-action: manipulation (or none) will work in the future, but doesn't currently.

Thanks!

(In reply to makeworld from comment #23)

My understanding from reading this issue is that setting minimum-scale=1 and maximum-scale=1 alone will fix this bug in Firefox 97 and later. Is that correct?

Yes, and also in Firefox 96 if the beta uplift request in comment 22 is approved (Firefox 96 is currently in beta).

What about user-scalable=no? Does setting that alone work?

From what I've read, setting touch-action: manipulation (or none) will work in the future, but doesn't currently.

These remain to be implemented. We ended up using this bug to track the regression in the behaviour of minimum-scale and maximum-scale, but we do still want to fix user-scalable and touch-action as discussed in comment 4. I'll file a follow-up for that.

Blocks: 1746126

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

I'll file a follow-up for that.

Filed bug 1746126.

Comment on attachment 9252098 [details]
Bug 1738696. Don't allow AsyncPanZoomController::ZoomToRect to change zoom to a value outside of zoom constraints. r?botond

Approved for 96.0b6

Attachment #9252098 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9252486 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: