1452825 - [scroll-behavior] not honored on history traversal

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[jonjohnjohnson]

Attachment: scroll-behavior and history traversal

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180404100127

Steps to reproduce:

"History Component" isn't available in the list, so "Document Navigation" is where I filed, though "Panning and Zooming" might be applicable as well.

1. Open test case
2. Click both the "top" and "bottom" links a few times noticing how the document smoothly scrolls between the top and bottom anchors.
3. Use the back and forward buttons in the browsers UI, notice how there is no smooth behavior.


Actual results:

No smooth scrolling when either using the browsers forward/back buttons OR programmatically interacting with the history interface, thought 'scroll-behavior: smooth' is set.


Expected results:

https://github.com/w3c/csswg-drafts/issues/2454

According to the spec and authors, all types of navigation, including history traversal should adhere to the value of the scroll-behavior property.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: wip

Need to write some test

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: history_scroll_with_test.diff

commit message would be,
"Bug 1452825, use smooth scrolling also for history navigations, r=bz"

The test is a tad ugly, but wpt wasn't playing too nicely with me so went with rather safe-and-dirty way. smoothHistoryNavigationTest fails without the patch.

remote: View your change here:
remote:   https://hg.mozilla.org/try/rev/918bc0971d39adfdf8f69282cf0b3cf263c1831f
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=918bc0971d39adfdf8f69282cf0b3cf263c1831f
remote: recorded changegroup in replication log in 0.070s

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8966583 [details] [diff] [review]
history_scroll_with_test.diff

ScrollTo() ends up checking ScrollBehaviorEnabled() already, so you shouldn't need to test it here, I would think...

We're going to need to rejigger this code some more when we fix bug 1442958, but that's ok.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

I was just following the setup done elsewhere.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

But I see we're inconsistent. I can drop the check.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Attachment: history_scroll_with_test_2.diff

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bbe699e731a
use smooth scrolling also for history navigations, r=bz

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/10472 for changes under testing/web-platform/tests

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/w3c/web-platform-tests/pull/10472
* continuous-integration/travis-ci/pr (https://travis-ci.org/w3c/web-platform-tests/builds/366535796?utm_source=github_status&utm_medium=notification)

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/8bbe699e731a

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged

Comment 12

[jonjohnjohnson]

This works beatutifully in Firefox Nightly 63.0a1 (2018-08-06)! Thanks!

Comment 13

[jonjohnjohnson]

(In reply to Olli Pettay [:smaug] from comment #1)

Actually I think I spoke too soon. :(

Though this issue is *mostly fixed, it seems to be fixed in a way that doesn't behave uniformly when new programmatic scrolling interactions overlap previous interactions?

Firefox Nightly 83.0a1 (2020-10-10) Mac 10.12.6

These two scenarios produce unspecified results in the original test case:

• Clicking an anchor link, then before the smooth scrolling behavior of that first anchor action finishes, click a different anchor link. This will cease the previous programmatic scrolling to the first anchor and instead scroll to the destination of the second anchor link. So far I believe this is both expected by users and matches specification. After these actions, now try to click on the browsers back button. Instead of it going back to the location of the first anchor, it will scroll you back to the scroll position when the second anchor had been clicked, all the while changing the hash of the url to the correct value, yet still being in inaccurate scroll position.
• Click both anchors back and forth a dozen times with enough time to scroll between both destinations. Then click on the browsers back and forward buttons quickly and repeatedly, you will eventually find that the page will stop at scroll positions not related either of the two anchors. Meaning scroll positions are being decided that do not at all match either of the hashes added to the url.

As far as user expectation goes for history traversal, I believe that when the hash of the url changes, the scroll position of the page will *eventually scroll a user to the position on the page that targets the value of the hash. Though these are edge cases, they are not what a user would expect and maybe this exposes some faulty logic underneath your implementation?

Comment 14

[jonjohnjohnson]

(In reply to Cosmin Sabou [:CosminS] from comment #10)

https://hg.mozilla.org/mozilla-central/rev/8bbe699e731a

Since my last comment may not have been seen, I figured I'd try to simplify.

AFAIKT, the behavior shown by the following steps is not what is expected by the spec.

1. Load the case -> https://bugzilla.mozilla.org/attachment.cgi?id=8966431
2. Immediately click the "bottom" link that's in the upper right up the page, see smooth scrolling behavior scroll the page down to the bottom.
3. Using your mouse or trackpad, scroll to an arbitrary position in the middle of the page.
4. Click the "top" link that's in the upper right of the page, see smooth scrolling behavior scroll the page up to the top.
5. Use the browsers "back button" to navigate back to the last hash (#bottom). But now see scrolling bring the user to the previously arbitrary position halfway down the page, even though the url has in fact changed to append the "#bottom".

This seems especially suspect, when the user can copy/paste the now current url into a new window and then see the window scrolled to a different view/target than would be expected from the previous experience using the history/back button to reach the same exact url.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

That is how I'd expect the behavior to be. Store scrolling position when doing another fragment navigation.