1334676 - Focusing a position:fixed input causes the page to scroll back up to the top.

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Keyboards and IME, defect)

Description

[Thomas Wisniewski]

Attachment: testcase.html

When an input with position:fixed (either on its own or in a container) is focused, the windows scrolls back up to the top of the page as the on-screen keyboard is brought up. This loses the user's place in the document, which can be frustrating. It doesn't seem to matter if the user focuses the element, or it is focused via script.

This issue does not occur in Chrome.

I've attached a simple test-case to demonstrate the issue. It only happens on a physical Android device, not in the responsive design view on a desktop version of Firefox, leading me to suspect that it may be related to the code which brings up the on-screen keyboard.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Randall can you take a look please

Comment 2

[Randall Barker [:rbarker]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #1)
> Randall can you take a look please

Our handling of fixed position stuff is not good. Try selecting text in a fixed element. Then scroll. Watch the handles scroll up the page.

Comment 3

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1334676-Prevent-nsDOMWindowUtils-ZoomToFocusedInput-from-panning-and-zooming-when-focused-element-is-fixed-position-17030915-54365dce91e1.patch

This patch just prevent nsDOMWindowUtils::ZoomToFocusedInput from zooming to focused element if it is fixed position. If the focused element ends up under the virtual keyboard, there is no way for the user to pan the content into view.

Comment 4

[Randall Barker [:rbarker]]

Bug 1123938 means our behavior will differ from Chrome. Since it is over two years old, I'm not sure it is going to get addressed anytime soon.

Comment 5

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8845674 [details] [diff] [review]
0001-Bug-1334676-Prevent-nsDOMWindowUtils-ZoomToFocusedInput-from-panning-and-zooming-when-focused-element-is-fixed-position-17030915-54365dce91e1.patch

You don't need scrolledFrame, you can just use rootScrollFrame.

And you probably want to stop at shell->GetRootFrame(). If we reach shell->GetRootFrame() then that means it is fixed pos content.

Comment 6

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1334676-Prevent-nsDOMWindowUtils-ZoomToFocusedInput-from-panning-and-zooming-when-focused-element-is-fixed-position-17030916-9f4c2b461667.patch

Addressed comments

Comment 7

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8845685 [details] [diff] [review]
0001-Bug-1334676-Prevent-nsDOMWindowUtils-ZoomToFocusedInput-from-panning-and-zooming-when-focused-element-is-fixed-position-17030916-9f4c2b461667.patch

I think it'd be safer if we flipped it so that isFixedPos started true, and we set it to false once we find out it is inside the scroll frame. That means the code also "works" if currentFrame is not a descendant of rootFrame (even though that is guaranteed in the current code).

Comment 8

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1334676-Prevent-nsDOMWindowUtils-ZoomToFocusedInput-from-panning-and-zooming-when-focused-element-is-fixed-position-r-tnikkel-17031011-c212f0d03959.patch

Addressed comments.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfa035314a3d
Prevent nsDOMWindowUtils::ZoomToFocusedInput from panning and zooming when focused element is fixed position r=tnikkel

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/dfa035314a3d

Comment 11

[Ryan VanderMeulen [:RyanVM]]

This is marked as tracking-fennec:54+. Are we intending to request uplift to Beta at some point or can this ride the 55 train?

Comment 12

[Randall Barker [:rbarker]]

I didn't realized it was tracking 54. I'll request uplift.

Comment 13

[Randall Barker [:rbarker]]

Comment on attachment 8846070 [details] [diff] [review]
0001-Bug-1334676-Prevent-nsDOMWindowUtils-ZoomToFocusedInput-from-panning-and-zooming-when-focused-element-is-fixed-position-r-tnikkel-17031011-c212f0d03959.patch

Approval Request Comment
[Feature/Bug causing the regression]: Zoom on element focus
[User impact if declined]: When a user focuses an element in a fixed element, the page gets panned to the top.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: it just short circuits the auto pan/zoom function on focused elements if the focused element is in a fixed element.
[String changes made/needed]: none

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8846070 [details] [diff] [review]
0001-Bug-1334676-Prevent-nsDOMWindowUtils-ZoomToFocusedInput-from-panning-and-zooming-when-focused-element-is-fixed-position-r-tnikkel-17031011-c212f0d03959.patch

54 is on Beta now :)

Comment 15

[Randall Barker [:rbarker]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #14)
> Comment on attachment 8846070 [details] [diff] [review]
> 0001-Bug-1334676-Prevent-nsDOMWindowUtils-ZoomToFocusedInput-from-panning-
> and-zooming-when-focused-element-is-fixed-position-r-tnikkel-17031011-
> c212f0d03959.patch
> 
> 54 is on Beta now :)

Sorry.

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8846070 [details] [diff] [review]
0001-Bug-1334676-Prevent-nsDOMWindowUtils-ZoomToFocusedInput-from-panning-and-zooming-when-focused-element-is-fixed-position-r-tnikkel-17031011-c212f0d03959.patch

This would go to beta (54) not aurora because of Project Dawn: https://hacks.mozilla.org/2017/04/simplifying-firefox-release-channels/

Let's uplift it now and it should end up in next week's beta 4 fennec build

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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