973851 - [Tracking] (reftest) tests in layout/reftests/position-sticky failed

Status: RESOLVED FIXED

(Testing :: Reftest, defect)

Description

[Abel Lin(alin, abel)]

Attachment: the difference of reftest case block-in-inline-3

Threre are two test failed when enabling OOP.
They are block-in-inline-2.html and block-in-inline-3.html,
which is related to APZ. The two tests would pass if APZ was desabled.

for example block-in-inline-3.html, by the attached, the scroll result from 20 to 60 is difference between APZ enabled and disabled.

I am not sure how APZ affect the tests directly or indirectly.

Comment 1

[Abel Lin(alin, abel)]

Hi Kartikaya,

Do you have any thoughts on the bug?
I have wrote a app like that as well.
The app has the sympton when enabling/disabling the APZ.
It should is correct(as the attached "NO APZ scroll60") rather than
wrong(as the attached "APZ scroll60").

For observing the rendering result, the sample was modified from block-in-inline-3.html.
See the sample-test.html.

Comment 2

[Abel Lin(alin, abel)]

Attachment: sample-test.html

Comment 3

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

Assuming that your scrolling is implemented as in the test case (i.e. by setting the scrollTop property on some div) then the APZ code itself shouldn't really be coming into play here, and shouldn't affect the test. Most likely this is a layout issue resulting from having a displayport set on some piece of content. You can ask some layout folks for help here but first you should probably verify that is what is going on. Comment out the implementation of nsDOMWindowUtils::SetDisplayPortForElement at [1] and see if that fixes the problem. If it does fix the problem then likely the problem is in the implementation of position:sticky in layout not respecting displayports properly.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDOMWindowUtils.cpp#356

Comment 4

[Abel Lin(alin, abel)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)
> Assuming that your scrolling is implemented as in the test case (i.e. by
> setting the scrollTop property on some div) then the APZ code itself
> shouldn't really be coming into play here, and shouldn't affect the test.
> Most likely this is a layout issue resulting from having a displayport set
> on some piece of content. You can ask some layout folks for help here but
> first you should probably verify that is what is going on. Comment out the
> implementation of nsDOMWindowUtils::SetDisplayPortForElement at [1] and see
> if that fixes the problem. If it does fix the problem then likely the
> problem is in the implementation of position:sticky in layout not respecting
> displayports properly.
> 
> [1]
> http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDOMWindowUtils.
> cpp#356
Thanks. I would look into it from here.

Comment 5

[Peter Chang[:pchang]]

abel, I found the wrong position problem related to position-sticky from nightly on linux.
It happened when you resize browser size smaller than the scroll frame size.

The height of visible region[1] of that position-sticky displayitem(nsDisplayText) was changed for every resize cycle.

And the height change was related to "ToReferenceFrame()".
But I'm not the issue I mentioned are related to this reftest fail or not.

[1]http://dxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp#2385
[2]http://dxr.mozilla.org/mozilla-central/source/layout/generic/nsTextFrame.cpp#4486

Comment 6

[Abel Lin(alin, abel)]

The wrong size calculation is in StickyScrollContainer::PositionContinuations. The value will be increased more and more. Eventually SetPostion reset inline frame, which result in the frame far away from beginning position.

Comment 7

[Abel Lin(alin, abel)]

The wrong size calculation is in StickyScrollContainer::PositionContinuations. The value will be increased more and more. Eventually SetPostion reset inline frame, which result in the frame far away from beginning position.

Comment 8

[Peter Chang[:pchang]]

Just confirmed that the wrong position problem I mentioned on comment 5 is the same root cause as  reftest fail items related to position-sticky.

Comment 9

[Peter Chang[:pchang]]

I created the following simple test case and pasted the mapped layout structure.
In [1], the (x,y) of block frame is always (0,0), but not for inline frame.
The height of inline frame("after block") is increased every time because of the addition with translation value.

Maybe we could fix it by reset the height of inline frame.

[Sample code]
 26   <body>
 27     <div id="scroll">
 28         <div id="sticky">
 29         <div>hello</div>after block
 30         </div>
 31     </div>

[Layout structure]
Block(div)(1)@7fd5a2cc0a98
	Inline(div)(1)@7fd5a2cc1118
		Text(0)"\n\t"
	Block(div)(1)@7fd5a2cc1868
		Block(div)(1)@7fd5a2cc1200
			Text(0)"hello"@7fd5a2cc1348
	Inline(div)(1)@7fd5a2cc19b0
		Text(2)"after block\n

[1]http://dxr.mozilla.org/mozilla-central/source/layout/generic/StickyScrollContainer.cpp#337

Comment 10

[Abel Lin(alin, abel)]

The inline failed to update, as below:
The dirty flag wasn't set(http://dxr.mozilla.org/mozilla-central/source/layout/generic/nsBlockFrame.cpp?from=nsBlockFrame.cpp&case=true#1901)
, so ReflowLine wasn't invoked(
http://dxr.mozilla.org/mozilla-central/source/layout/generic/nsBlockFrame.cpp?from=nsBlockFrame.cpp&case=true#2021), which result in wrong calculation of inline.

In normal cases(reload button pushed), the dirty flag is always set every time.
But abnormal case(resize), the dirty flag is not set the last time.

Comment 11

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

All the reftests in https://searchfox.org/mozilla-central/rev/26b13464c2beb26e0d864d561c30e817a85c348a/layout/reftests/position-sticky/reftest.list are pass (although some of them have fuzzy annotation).