Closed Bug 1334676 Opened 3 years ago Closed 3 years ago

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

Categories

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

Unspecified
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
fennec 54+ ---
firefox53 --- wontfix
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: wisniewskit, Assigned: rbarker)

References

Details

(Whiteboard: [webcompat])

Attachments

(2 files, 2 obsolete files)

Attached file 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.
Randall can you take a look please
Assignee: nobody → rbarker
tracking-fennec: ? → 54+
Flags: needinfo?(rbarker)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) 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.
Flags: needinfo?(rbarker)
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.
Depends on: viewport-compat
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 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 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).
Attachment #8846070 - Flags: review?(tnikkel) → review+
Pushed by rbarker@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfa035314a3d
Prevent nsDOMWindowUtils::ZoomToFocusedInput from panning and zooming when focused element is fixed position r=tnikkel
https://hg.mozilla.org/mozilla-central/rev/dfa035314a3d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
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?
Flags: needinfo?(rbarker)
I didn't realized it was tracking 54. I'll request uplift.
Flags: needinfo?(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
Attachment #8846070 - Flags: approval-mozilla-aurora?
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 :)
Attachment #8846070 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
(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 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
Attachment #8846070 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 656036
No longer depends on: viewport-compat
You need to log in before you can comment on or make changes to this bug.