The Fullscreen API spec states that top layer elements can have position value 'absolute' or 'fixed', but currently we only support 'fixed' because we only need that for fullscreen. However, we eventually want to also support 'absolute' for <dialog>.

It seems a naive change in css frame constructor and rule node doesn't work as expected. Scrolling via mouse wheel doesn't move the absolutely-positioned top layer element. I guess there is some layer-related optimization kicks in, but do not have idea what it is and how can I fix it. Matt, could you provide some information around this? Currently the display lists of all top layer elements are built in the viewport frame to ensure their painting order, however the containing block of absolutely-positioned elements is the initial containing block. I guess building the display lists in canvas frame could fix this, but I suspect that may violate the rendering order.

If we want elements within the top layer to be scrollable, then we probably need those frames to be children of the root scrollframe. We compute the on-screen position of elements by walking up the frame ancestors and adding the top/left of each one. We need the scrollable frame to be in this chain in order to get scroll position taken into account. Can we have the frames there, and then fix the display list sorting code to make sure the top level content ends up in the right place?

They are already children of the initial containing block (the canvas frame I suppose), so should be descendent of the root scrollframe. It is just that their display list are built by the viewport frame rather than their containing block. When I drag the scrollbar, the absolutely-positioned top layer element moves as expected, but when I use the mouse wheel, the element is not moved until a next paint, and a white area is scrolled instead. So it seems there is some compositing optimization for scrolling?

Does it work if you disable layers.async-pan-zoom.enabled (needs a restart), or if you disable e10s? If so, then it's probably a problem in the way we annotate layers to tell APZ how to scroll them.

Yeah, it works with either layers.async-pan-zoom.enabled or e10s disabled. So, it seems yes, it is something related to APZ. That means I would need to put them in a layer in the first place? Is there any document or could you point me to some source file about working with APZ?

There isn't really, sorry. You shouldn't really need explicit layers as such. Each display item has an 'animated geometry root' (AGR) which is the topmost frame that we expect that display item to stay fixed relative to (for example, an ancestor with animated left, or the root frame in a scroll container). FrameLayerBuilder creates separate layers for each set of consecutive display items with the same AGR, so we'd expect scrolled top layer content and fixed top layer content to be split into separate layers already. We then tag these layers with FrameMetrics object that describe the scrollable containers that contain them, so that APZ knows how to handle them. Can you enable layout.display-list.dump in a debug build, and then upload the dump somewhere. It'll dump for every paint (and for both processes), so expect a lot of output. If you enable the pref, load the broken content and then kill the process it should be easy to find the relevant dump at the end.

This should be the relevant log. The BackgroundColor at the last line is the absolutely-positioned top layer element (id:top-layer-test）, and its parent wrapper is the wrap list for all items in the top layer. From this dump, the most obvious difference between that item and other scrollable items is the value inside scrollClip(). Other items have scrollClip(<0,5100,75060,12900> [async-scrollable]) while that of the top layer item is just scrollClip(). This could be the reason.

You can use id:top-layer-test to locate the target display item.

You are correct :) The scroll clip (chain) attached to a display item is how FrameLayerBuilder finds the scroll frames that might scroll a given item. We add an entry to the chain when we go through ScrollFrameHelper::BuildDisplayList so that descendant items know they are scrollable. The custom display list building code that bypasses this isn't getting the scroll clips setup, so we don't have any scrolling information. CC'ing Markus, since he's the scroll clip expert :)

I think we need to build the top layer display items from within ScrollFrameHelper::BuildDisplayList (at least the ones that are attached to that frame), and then fix display list sorting to get them back to the right place. The other option is factoring out all the scroll clip setup code so we can make sure the same code runs when doing the custom display list building in ViewportFrame, but that seems more error prone.

Review commit: https://reviewboard.mozilla.org/r/44543/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/44543/

Review commit: https://reviewboard.mozilla.org/r/44545/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/44545/

Review commit: https://reviewboard.mozilla.org/r/44547/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/44547/

Comment on attachment 8738443 [details] MozReview Request: Bug 1236828 part 1 - Apply proper clip state to top layer frames. r=mstange https://reviewboard.mozilla.org/r/44543/#review41337 ::: layout/generic/nsViewportFrame.cpp (Diff revision 1) > if (!(frame->GetStateBits() & NS_FRAME_OUT_OF_FLOW)) { > MOZ_ASSERT(!elem->GetParent()->IsHTMLElement(), "HTML element " > "should always be out-of-flow if in the top layer"); > continue; > } > - MOZ_ASSERT(frame->GetParent() == this); Is this assert wrong? Why?

Comment on attachment 8738444 [details] MozReview Request: Bug 1236828 part 2 - Make frame constructor support absolutely-positioned top layer frame. r=bz https://reviewboard.mozilla.org/r/44545/#review41385 ::: layout/base/nsCSSFrameConstructor.cpp:761 (Diff revision 1) > > // Containing block information for out-of-flow frames. > nsAbsoluteItems mFixedItems; > nsAbsoluteItems mAbsoluteItems; > nsAbsoluteItems mFloatedItems; > - // Items in the top layer are always fixed positioned children of the > + // The containing block of frame in the top layer is defined by the "of a frame" ::: layout/base/nsCSSFrameConstructor.cpp:762 (Diff revision 1) > // Containing block information for out-of-flow frames. > nsAbsoluteItems mFixedItems; > nsAbsoluteItems mAbsoluteItems; > nsAbsoluteItems mFloatedItems; > - // Items in the top layer are always fixed positioned children of the > - // viewport frame. It differs from mFixedItems that the items here > + // The containing block of frame in the top layer is defined by the > + // spec: fixed-positioned frames are children of viewport frame, and "of the viewport frame" ::: layout/base/nsCSSFrameConstructor.cpp:1140 (Diff revision 1) > MOZ_ASSERT(aStyleDisplay->IsAbsolutelyPositionedStyle(), > "Top layer items should always be absolutely positioned"); > - return mTopLayerItems.containingBlock; > + if (aStyleDisplay->mPosition == NS_STYLE_POSITION_FIXED) { > + MOZ_ASSERT(mTopLayerFixedItems.containingBlock, "No root frame?"); > + return mTopLayerFixedItems.containingBlock; > + } else { Please lose the else-after-return. r=me

https://reviewboard.mozilla.org/r/44543/#review41337 > Is this assert wrong? Why? Hmmm, this actually should belong to part 2. With part 2, top layer frames would be child of the canvas frame when they have "position: absolute" rather than fixed, and thus this assert no longer valid.

It seems dbaron currently has a great backlog on reviewing. As this is a trivial change on style part, I'd try heycam. Hmmm, it seems heycam has some backlog there as well, but might be slightly better than dbaron :)

Comment on attachment 8738445 [details] MozReview Request: Bug 1236828 part 3 - Allow setting position: absolute for top layer element and add test. r=heycam https://reviewboard.mozilla.org/r/44547/#review41939 Can you add a test for :-moz-full-screen::backdrop { position: absolute; }?

Then I found a crash :/

Comment on attachment 8738443 [details] MozReview Request: Bug 1236828 part 1 - Apply proper clip state to top layer frames. r=mstange Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44543/diff/1-2/

Comment on attachment 8738444 [details] MozReview Request: Bug 1236828 part 2 - Make frame constructor support absolutely-positioned top layer frame. r=bz Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44545/diff/1-2/

Comment on attachment 8738445 [details] MozReview Request: Bug 1236828 part 3 - Allow setting position: absolute for top layer element and add test. r=heycam Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44547/diff/1-2/

Comment on attachment 8738445 [details] MozReview Request: Bug 1236828 part 3 - Allow setting position: absolute for top layer element and add test. r=heycam https://reviewboard.mozilla.org/r/44547/#review41955 While I don't think it's needed for this bug's patches specifically, it would be good to have a test for ::backdrop with top/right/bottom/left values that don't cover the entire viewport, too. ::: dom/html/test/file_fullscreen-backdrop.html:95 (Diff revision 2) > + setBackdropStyle("position: absolute"); > + info("Changing position shouldn't immediately affect the view"); > + assertWindowPureColor(window, "black"); > + > + window.scroll(0, screen.height); > + info("Scrolled up the absolutely-positioned absolute"); "Scrolled up the absolutely-positioned element"?

Comment on attachment 8738444 [details] MozReview Request: Bug 1236828 part 2 - Make frame constructor support absolutely-positioned top layer frame. r=bz https://reviewboard.mozilla.org/r/44545/#review42063 r=me, assuming the only substantive change here is which framelist the backdrop placeholder goes in.

Comment on attachment 8738443 [details] MozReview Request: Bug 1236828 part 1 - Apply proper clip state to top layer frames. r=mstange Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44543/diff/2-3/

Comment on attachment 8738444 [details] MozReview Request: Bug 1236828 part 2 - Make frame constructor support absolutely-positioned top layer frame. r=bz Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44545/diff/2-3/

Comment on attachment 8738445 [details] MozReview Request: Bug 1236828 part 3 - Allow setting position: absolute for top layer element and add test. r=heycam Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44547/diff/2-3/

https://hg.mozilla.org/integration/mozilla-inbound/rev/b7f833f9fe09 https://hg.mozilla.org/integration/mozilla-inbound/rev/5e846f999e8b https://hg.mozilla.org/integration/mozilla-inbound/rev/f6e275dfcdee

https://hg.mozilla.org/mozilla-central/rev/b7f833f9fe09 https://hg.mozilla.org/mozilla-central/rev/5e846f999e8b https://hg.mozilla.org/mozilla-central/rev/f6e275dfcdee