element positioned:fixed on bottom may disappear
Categories
(Core :: Panning and Zooming, defect, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox105 | --- | fixed |
People
(Reporter: dlrobertson, Assigned: dlrobertson)
References
Details
Attachments
(2 files)
Steps to reproduce:
- on a android device with the toolbar set at the bottom
- navigate to a page with a
position: fixedelement that is smaller than the height of the bottom bar (See the example) - scroll down to collapse the bottom toolbar
- click the element several times
Expected results:
- The button still exists
Actual Results:
- The button may disappear.
|
Assignee | |
Comment 1•2 years ago
|
||
|
||
Comment 2•2 years ago
|
||
Thanks for filing! This looks pretty bad, potentially a candidate for one of our upcoming Android sprints.
|
|
Assignee | |
Comment 3•2 years ago
|
||
Did a little debugging on this today and after a first pass, it appears the bottom element in the testcase doesn't exist in mFixedPositionInfo, so SampleForWebRender will not adjust the fixed content to a visible position.
|
|
Assignee | |
Comment 4•2 years ago
|
||
(In reply to Dan Robertson (:dlrobertson) from comment #3)
Did a little debugging on this today and after a first pass, it appears the bottom element in the testcase doesn't exist in mFixedPositionInfo, so SampleForWebRender will not adjust the fixed content to a visible position.
After some more debugging, this seems to occur because the element fixed to the bottom doesn't end up making it into the display list, and only happens when the toolbar is completely hidden. This makes me wonder if there is a spot in the building of the display list where the visual viewport size is not the result of GetVisualViewportSizeUpdatedByDynamicToolbar().
|
|
||
Comment 5•2 years ago
|
||
(In reply to Dan Robertson (:dlrobertson) from comment #4)
After some more debugging, this seems to occur because the element fixed to the bottom doesn't end up making it into the display list, and only happens when the toolbar is completely hidden. This makes me wonder if there is a spot in the building of the display list where the visual viewport size is not the result of
GetVisualViewportSizeUpdatedByDynamicToolbar().
Interesting finding! Display list building skipping the fixed element because the visible/dirty rect is not sized to include the dynamic toolbar does sound like a plausible explanation.
I think a relevant place to look further is the DescendIntoChild() call in BuildDisplayListForChild() (and the similar call in BuildDisplayListForSimpleChild()), to see if we fail to descend into the fixed element as a result of this check. The implementation of this function checks if the frame in question intersects the visible and dirty rects here.
|
||
Comment 6•2 years ago
|
||
Since this bug is about a position fixed item one thing that may be important is how we handle he dirty rects for out of flows like fixed pos.
When we enter the geometric parent frame during display list building we call MarkAbsoluteFramesForDisplayList
and that stores a dirty rect for all absolute children (and fixed = absolute frames on the viewport).
And then when we descend into the frame via it's content parent frame we retrieve the stored dirty rect here
and thats when we actually build the display items. It's done like this because of how css defines painting order.
The function nsDisplayListBuilder::OutOfFlowDisplayData::ComputeVisibleRectForFrame looks like it could be involved.
|
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Comment 7•2 years ago
|
||
I'm starting to get a better idea of what is going on. Still a bit fuzzy on the details, but this only happens for a partial display list build. This appears to be a regression from bug 1650438. Due to ComputeVisibleRectForFrame not including the fixed position content, it doesn't make it into the partial display list and the the FixedPositionInfo is lost here. I think there are a few routes forward:
- Do not clear the
APZCManager::mFixedPositionInfowhen we receive a partial display list build. Instead update the info stored with any new or updated values. - Revert or reduce the impact of the patch to fix bug 1650438. Simple local tests pass and the fixed position elements no longer disappear.
|
|
||
Comment 8•2 years ago
|
||
One simple approach along the lines of (2) to consider could be, "force a full display list rebuild when the toolbar transitions from not-fully-hidden to fully-hidden".
The alternative would be to make partial updates correctly handle that transition. Not clearing mFixedPositionInfo on a partial update might be one of things needed to make that work, but I'm not sure whether other pieces would be required as well.
|
|
Assignee | |
Comment 9•2 years ago
|
||
One simple approach along the lines of (2) to consider could be, "force a full display list rebuild when the toolbar transitions from not-fully-hidden to fully-hidden".
This is a good point! I wonder if there are other cases where us not forcing a full display list rebuild on toolbar transitions could be a source of buggyness.
|
|
||
Comment 10•2 years ago
|
||
This is an existing place where we do a similar check for "something changed that requires a full update".
|
|
Assignee | |
Comment 11•2 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #10)
This is an existing place where we do a similar check for "something changed that requires a full update".
Hmmmm forcing a full update on the dynamic toolbar state change doesn't seem to fix the issue. I'll have to do some more digging on how partial updates work.
|
|
Assignee | |
Comment 12•2 years ago
|
||
I've done a lot more debugging on this over the past few days, and while the following suggestion does make the fixed box disappear less, it does not fix the issue.
(In reply to Botond Ballo [:botond] from comment #8)
One simple approach along the lines of (2) to consider could be, "force a full display list rebuild when the toolbar transitions from not-fully-hidden to fully-hidden".
After more debugging it appears there is some sort of error in the calculation of the dirty area when the toolbar is collapsed. So the fact that this only occurs with a partial update is sort of a red herring. For the example provided, when the button fixed to the bottom is clicked the display list builders current mDirtyRect is always (0, 0, 0, 0) for me, but when the toolbar is in transition (even when almost entirely collapsed), the mDirtyRect is populated with a value that makes a lot more sense.
|
|
||
Comment 13•2 years ago
|
||
(In reply to Dan Robertson (:dlrobertson) from comment #12)
After more debugging it appears there is some sort of error in the calculation of the dirty area when the toolbar is collapsed. So the fact that this only occurs with a partial update is sort of a red herring. For the example provided, when the button fixed to the bottom is clicked the display list builders current
mDirtyRectis always(0, 0, 0, 0)for me, but when the toolbar is in transition (even when almost entirely collapsed), themDirtyRectis populated with a value that makes a lot more sense.
This is the dirty rect inside the fixed subtree? Have you tried looking at the values in the places in comment 6? That, and related code, is where I would expect the dirty rect to be determined for the fixed subtree.
|
|
Assignee | |
Comment 14•2 years ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #13)
This is the dirty rect inside the fixed subtree? Have you tried looking at the values in the places in comment 6? That, and related code, is where I would expect the dirty rect to be determined for the fixed subtree.
Yeah, the dirty rect handling here does seem off. I just haven't figured out how yet. I did note that we never hit this in the fixed-pos + collapsed toolbar case.
|
|
Assignee | |
Comment 15•2 years ago
|
||
When the dynamic toolbar is completely collapsed and calculating the visible
rect for out of flow content, ensure that the visible rect is expanded to
include the dynamic toolbar max height.
|
||
Comment 18•2 years ago
|
||
Pushed by drobertson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fae7a5651bb9 Expand visible rect height if dynamic toolbar is collapsed. r=tnikkel
|
||
Comment 19•2 years ago
|
||
| bugherder | ||
Description
•