1909712 - about:addons scroll restoration is not waiting for all content to load properly

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

After bug 1909048, we follow the spec with regards to timing of requestAnimationFrame, but browser_html_scroll_restoration.js is failing one subtest somewhat often, so I'm going to disable it to avoid the back out for now, since I'm pretty sure it's not a real regression.

The issue seems to be that, at the time we call ScrollOffsets.restore() which is in the second rAF callback (here), all the content is still not there. So since we're scrolled all the way to the bottom, we get clamped, then the content arrives.

We should fix the restoration to either:

• Wait reliably for the content to be there.
• Retry it once the content is there (assuming we haven't scrolled).
• Adjust the test.

I'm attaching logs that show the difference in layout from the point the scrollTo() call is made, to the point where the test checks the scroll offset, which shows the difference quite clearly.

Mark, you touched this last in bug 1843148, is there any reason there's supposed to be any guarantee of the content being there in two rAFs?

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Frame tree when the scrollTo call is made.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Frame tree when the test checks the offsets (content already loaded)

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: diff between the two logs.

The recommended section footer is empty when we try to restore the scroll position it seems. Text not localized yet?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: More readable diff.

(Removed some whitespace)

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1909712 - Ensure there are no pending translations before restoring the scroll position. r=robwu,mstriemer

Locally the second rAF doesn't seem to be needed, I could probably
remove it.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd7fbac88c03
Ensure there are no pending translations before restoring the scroll position. r=robwu

Comment 7

[Serban Stanca [:SerbanS]]

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

Comment 9

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

Set release status flags based on info from the regressing bug 1909048

Comment 10

[Intermittent Failures Robot]

7 failures in 4124 pushes (0.002 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 5
• mozilla-central: 2

Platform and build breakdown:

• linux1804-64-qr: 4
  • opt: 4
• linux1804-64-shippable-qr: 3
  • opt: 3

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1909712&startday=2024-07-22&endday=2024-07-28&tree=all