1526609 - Scroll position is lost on reflow with large enough scroller.

Status: VERIFIED FIXED

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

Description

[Emilio Cobos Álvarez [:emilio]]

Attachment: t.html

STR:

• Open the test-case.
• Scroll somewhere down the middle.
• Resize the window.

Expected:

• Scroll position isn't lost.

Actual:

• Scroll jumps to the top.

This is the root cause of https://github.com/webcompat/web-bugs/issues/22588.

The amount of content does matter, apparently.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Will take a closer look on Monday. If someone could run a regression range it'd be lovely, just to confirm whether it's a regression or not.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Seems to happen only when shrinking, which probably means that this is related to us switching scrollbar choice between choosing only vertical scrollbars or both scrollbars.

This is probably what can cause the issue with flex layouts more easily. It's easy to imagine a flex item that doesn't get a vertical scrollbar when measured and does on the final reflow, or something of that sort.

Comment 3

[Cameron McCormack (:heycam)]

mozregression gave me this:

16:19.13 INFO: Last good revision: 7efda263a842e60cd0cc00b3c4a7058c65590702 (2017-06-08)
16:19.13 INFO: First bad revision: f4262773c4331d4ae139be536ce278ea9aad3436 (2017-06-09)

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7efda263a842e60cd0cc00b3c4a7058c65590702&tochange=f4262773c4331d4ae139be536ce278ea9aad3436

Nothing is standing out as obvious in there.

Comment 4

[Cameron McCormack (:heycam)]

OK, git bisect leads me to believe the regression was introduced in bug 1370072.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Huh, what? That is quite surprising.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Got it.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1526609 - When interrupting the reflow of abspos kids, still account for the old overflow areas.

Otherwise, if we're scrolled, the scroll frame will think that our scroll range
was reduced, which will in turn clamp the scroll position, which is not good.

I... don't really know how to add a test for this interruptible reflow stuff.
Ideas?

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Possibly useful: https://searchfox.org/mozilla-central/rev/5e3bffe964110b8d1885b4236b8abd5c94d8f609/gfx/layers/apz/test/mochitest/test_interrupted_reflow.html#619-626

Comment 9

[Emilio Cobos Álvarez [:emilio]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #8)

Possibly useful: https://searchfox.org/mozilla-central/rev/5e3bffe964110b8d1885b4236b8abd5c94d8f609/gfx/layers/apz/test/mochitest/test_interrupted_reflow.html#619-626

Thanks! I managed to get a regression test working cargo-culting from that one :)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/35025f167143
When interrupting the reflow of abspos kids, still account for the old overflow areas. r=bzbarsky

Comment 11

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/35025f167143

Comment 12

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 9042970 [details]
Bug 1526609 - When interrupting the reflow of abspos kids, still account for the old overflow areas.

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

N/A

User impact if declined

Scroll position is lost sometimes. This affects outlook.com.

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 STR in https://github.com/webcompat/web-bugs/issues/22588.

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Pretty low-risk / simple fix to avoid loosing the scroll range if our reflow gets interrupted.

String changes made/needed

none

Comment 13

[Cristian Craciun]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:67.0) Gecko/20100101 Firefox/67.0
Build ID: 20190220040540

The issue is not completely fixed. After scrolling to the middle of the page and resizing the window, the scroll position is moved at the middle of the distance between the beginning of the page and the previous position of the scroll.

Please note that the second part of the issue is fixed. When the window is resized back to the default size, the scroll position goes back to the middle of the page.

Comment 14

[Emilio Cobos Álvarez [:emilio]]

It is expected that the exact element that is at the scroll position isn't preserved. The scroll distance from the top is what's preserved, and when you shrink the window that scroll position ends up pointing somewhere in the middle.

Of course, if you enlarge the window again the scroll position should be roughly the same again, can you confirm that's what happens?

Comment 15

[Cristian Craciun]

It's not about the scroll position that isn't preserved, the position of the content from the page is not preserved.

I've tried a different STR:

1. Scroll to the end of the page and notice that the background is green.
2. Resize the browser window to half.
3. Notice that the scroll position is at the middle of the page and the background is red now.

Could you confirm that this is the expected behavior?

And yes, I can confirm that when the window is enlarged to the default size, the scroll position and the content position are the same again.

Comment 16

[Emilio Cobos Álvarez [:emilio]]

Yes, that's expected. What wouldn't be expected is that position goes back to zero and doesn't go back to the original position when you resize back.

Comment 17

[Cristian Craciun]

Verified as fixed on the latest Nightly build based on Comment 16.

Comment 18

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

Comment on attachment 9042970 [details]
Bug 1526609 - When interrupting the reflow of abspos kids, still account for the old overflow areas.

Fix for a scroll position issue, verified in nightly.
let's uplift for beta 11.

Comment 19

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3c75057cd305

Comment 20

[Cristian Craciun]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:66.0) Gecko/20100101 Firefox/66.0
Build ID: 20190225143245

Verified as fixed on the latest Beta build (66b11).