1586909 - m.twitch.tv infinitely scrolls

Status: RESOLVED FIXED

(Core :: Layout: Scrolling and Overflow, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

See bug 1460032 comment 5. I didn't reproduce it on desktop though.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

STR:

• open m.twitch.tv (on desktop also works)
• Shrink the viewport.
• Scroll down.

ER: Scrolls down and doesn't jump

AR: Keeps scrolling down.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1586909 - Process anchor suppressions even when ignoring adjustments.

Otherwise we may keep the scroll anchor around and we may try to anchor to it
later even though we should've really suppressed it completely.

Maybe we should just call InvalidateAnchor() unconditionally from that code
path, even if there are no suppressions pending...

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/befaa1098701
Process anchor suppressions even when ignoring adjustments. r=dholbert

Comment 4

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

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

Comment 5

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

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/19561
* Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/Y__2buepSHa2xG1qXpTViA)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/613e9a1a99f3
followup: Appease wptlint.

Comment 7

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

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 8

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/befaa1098701
https://hg.mozilla.org/mozilla-central/rev/613e9a1a99f3

Comment 9

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

Upstream PR merged by moz-wptsync-bot

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9099420 [details]
Bug 1586909 - Process anchor suppressions even when ignoring adjustments.

Beta/Release Uplift Approval Request

• User impact if declined: See this bug and the duplicate.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0 or the duplicate.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple scroll anchoring fix to apply scroll anchoring less. This was a long-standing bug, but duplicate was regressed in 70.
• String changes made/needed: none

Comment 12

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

Comment on attachment 9099420 [details]
Bug 1586909 - Process anchor suppressions even when ignoring adjustments.

Fix for regression affecting popular site, verified in Nightly.
OK for uplift for beta 14.

Comment 13

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

[Tracking Requested - why for this release]:

Oana, or Stefan, is this something you or your team could test on Fennec?
This affects desktop but may also affect Fennec as the original issue was on the mobile site. Thanks!

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

Got conflicts while trying to uplift.

Emilio, could you, please, provide a updated patch?

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Solved on IRC, patches apply cleanly.

Comment 16

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-beta/rev/871b65946a55
https://hg.mozilla.org/releases/mozilla-beta/rev/0166812024e3

Comment 17

[Stefan Deiac]

(In reply to Liz Henry (:lizzard) from comment #13)

[Tracking Requested - why for this release]:

Oana, or Stefan, is this something you or your team could test on Fennec?
This affects desktop but may also affect Fennec as the original issue was on the mobile site. Thanks!

This issue is reproducible on Beta 68.2b6.
Devices:

• LG G7 fit (Android 8.1);
• OnePlus 5t (Android 9);
• Samsung Galaxy S8 (Android 9);
• Google Pixel 2 (Android 9).

Comment 18

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

Emilio can you also provide a patch for esr68?

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Attachment: ESR68 patch

The patch applies almost cleanly, except for the fact that bug 1561450 is not on ESR.

We should test that this patch actually fixes the issue on ESR as other scroll anchoring fixes have landed in 69+, but this patch should be harmless either way.

Comment 20

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

Comment on attachment 9100392 [details] [diff] [review]
ESR68 patch

Let's uplift to esr68, then verify on next week's fennec build and on ESR desktop as well.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/18ed5d5a087d

Comment 22

[Stefan Deiac]

Hello, I tried to reproduce the problem on Beta 68.2b7 and I can confirm that the quick scroll issue is not reproducible.
Devices:

• Nokia 6 (Android 7.1.1);
• Samsung Galaxy Note 8 (Android 9);
• Google Pixel (Android 9).

Due to my findings, I'll mark the 68esr flag as verified, thanks.

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 24

[Sorina Florean [:sflorean]]

Confirm that this also is fixed for 68.2.0 build 2 with Nexus 5 (Android 6.0.1) and LG G7 fit (Android 8.1). I will add again qe-verify+ flag for desktop testing.

Comment 25

[Hani Yacoub, Desktop QA]

I couldn't reproduce this issue on desktop on Firefox Nightly 71.0a1 (2019-10-07) and other old versions, I tried on both Windows 10 x64 and on Mac OS X 10.14.
Could someone provide extra tips on how to reproduce this issue on desktop please?

Comment 26

[Emilio Cobos Álvarez (:emilio)]

comment 1 is all I got, but seems like the site has been redesigned in the meantime so probably no longer reproduces.