Closed Bug 1627986 Opened 4 years ago Closed 4 years ago

Some sites with percentage height set on <html> can lose scroll position when moving focus from address bar

Categories

(Core :: DOM: UI Events & Focus Handling, defect, P2)

Product:
Core
Component:
DOM: UI Events & Focus Handling
Version:
75 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla77
Tracking Flags:
Tracking Status
firefox77 --- verified

People

(Reporter: quasicomputational, Assigned: emilio)

Details

Attachments

(5 files)

minimised.html
12.51 KB, text/html
Details
Stack of scrolling.
5.30 KB, text/plain
Details
Test-case with programmatic focus()
464 bytes, text/html
Details
Bug 1627986 - Minor cleanup in the document navigation code. r=hsivonen
47 bytes, text/x-phabricator-request
Details | Review
Bug 1627986 - Don't scroll when focusing the root for document navigation. r=hsivonen
47 bytes, text/x-phabricator-request
Details | Review

Steps to reproduce:

  1. Ensure browser.urlbar.openViewOnFocus is set to true.
  2. Scroll down.
  3. Move focus to the address bar by pressing F6.
  4. Move focus away from the address bar by pressing F6 again.

Expected behaviour: Scroll position is preserved.

Observed behaviour: The page is scrolled to the top.

I do see the expected behaviour with openViewOnFocus set to false (and browser.urlbar.update1 set to true).

Ah, this doesn't happen on every site. Here's a (relevant!) page that does show the loses-scroll-position behaviour: https://old.reddit.com/r/firefox/comments/fwguqj/here_is_what_is_new_and_changed_in_firefox_750/

It also doesn't seem to be related to either openViewOnFocus or update1 at all - I'm seeing the bad scrolling even with browser.urlbar.update1, browser.urlbar.openViewOnFocus and all children disabled, so it's possibly some kind of general regression in 75 - I'm pretty sure I'd've noticed this on 74, though I can't be sure now.

Summary: openViewOnFocus=true leads to losing scroll position when moving focus from address bar → After upgrading to 75, some sites lose scroll position when moving focus from address bar

I found another reproducer, and it's (a) from an amusing source, (b) less, and unminified, JS to dig through, and (c) a bit weirder: https://hacks.mozilla.org/2020/04/firefox-75-ambitions-for-april/

To get this one to lose scroll position, go through these steps:

  1. Scroll the page using any of PgDn, manipulating the scrollbar, or the scroll wheel at least once - arrow keys alone don't suffice.
  2. Focus the address bar, either by clicking on it or pressing F6.
  3. Shift focus back to the page by pressing F6 - clicking on the page retains scroll position.

By selectively toggling script origins on and off with uMatrix, I've narrowed it down to something first-party. It does seem to require some script - disabling scripts totally makes the bad behaviour vanish.

I had a look through the first-party scripts being run and didn't find any obvious candidates to explain the loss of scroll position.

Next, I tried enabling logging and breakpoints for all event listeners in the debugger, and went through the reproduction steps. I saw the bad behaviour, but nothing got logged to the console and no breakpoints fired, so I continue to think it's some kind of weird Firefox bug instead of the page's fault.

Attached file minimised.html —

OK, I've got reproducing it nailed down: the trigger is having html { height: 100% } - no script required! Definitely a Firefox bug.

The attachment is a minimised reproducer. I grabbed some text from a public domain work (Oliver Twist) to make a page a few screens long. I think the thing about arrow keys was that I had to be scrolled down more than a full screen for it to trigger.

Reproduction instructions:

  1. Open the attached file.
  2. Scroll down to the bottom.
  3. Focus the address bar, by clicking on it or pressing F6.
  4. Press F6 again.

I don't think this is an Address Bar bug, it sounds related to scrolling on focus.

Component: Address Bar → Layout: Scrolling and Overflow
Product: Firefox → Core

Thank you quasicomputational for reporting this bug.

The minimized reproducer attached in comment 3 is super helpful to nail down the issue! By using mozregression, I can reproduce this bug with it on Firefox 56, so this is definitely not an recent regression.

To layout engineers: This can be reproduced in layout debugger.

Priority: -- → P3

Move the component to "Panning and Zooming" since this is more an issue that we lost the scroll position of the root element when changing focus, not related to the rendering of the overflow area.

Component: Layout: Scrolling and Overflow → Panning and Zooming
Summary: After upgrading to 75, some sites lose scroll position when moving focus from address bar → Some sites with percentage height set on <html> can lose scroll position when moving focus from address bar

The least I can do when I see a nicely-reduced test-case like this is taking a look :-)

I think this is a bit unexpected but I'm not quite sure what the right fix is. I don't think this is an APZ bug.

The scrolling happens because that's handled like tab navigation.

Usually tab navigation / focus() only scrolls if not already visible. In this case the box for the <html> element is outside of the viewport, so we scroll all the way up...

Attached file Stack of scrolling. —

(Just for reference)

It seems we disagree with other browsers in how to handle this as well.

There are different things that we could do here...

  • Special-case document navigation to never scroll the root element.
  • Special-case the root element as "always visible" for the purposes of scrolling.
  • Take into account overflow areas for the visibility check.

From this test-case, Chrome seems to be doing (1), or some variation since <html> doesn't seem focusable in Chrome (document.activeElement remains the body element, not the root element).

Chrome behaves like Firefox if you do document.documentElement.scrollIntoView() (which sorta makes sense).

Based on the above, and on the assumption that we don't want to more generally change how scrollIntoView works, I'll move this to the Focus component.

There are two questions:

  • Should the HTML element be focusable in the first place? (That is, should document.activeElement == document.documentElement be true when you cal document.documentElement.focus()?). I don't have a great answer for that.
  • Should document navigation scroll the page? The answer for this is "probably not".
Component: Panning and Zooming → DOM: UI Events & Focus Handling

Just use early returns and such to simplify a bit.

Assignee: nobody → emilio
Status: NEW → ASSIGNED

This still scrolls back to the top when focusing the document via the tab key or
such... I've decided to preserve behavior there but let me know if you just want
me to add a root element check in nsFocusManager::ScrollIntoView instead.

Depends on D70539

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Priority: P3 → --
Priority: -- → P2
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b3608d9d7bd3
Minor cleanup in the document navigation code. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/a92516514d8c
Don't scroll when focusing the root for document navigation. r=hsivonen

https://hg.mozilla.org/mozilla-central/rev/b3608d9d7bd3
https://hg.mozilla.org/mozilla-central/rev/a92516514d8c

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox77: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla77

I verified this issue using Fx 77.0a1 (2020-04-29), on Windows 10 x64.

Status: RESOLVED → VERIFIED
status-firefox77: fixed → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: