We used to show them before r287683, just at the wrong place, and with the wrong size. Now, we no longer show them. Example testcases are: imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/top-layer-containing-block.html [ ImageOnlyFailure ] imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/top-layer-parent-overflow-clip.html [ ImageOnlyFailure ] imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/top-layer-parent-overflow-scroll.html [ ImageOnlyFailure ]
<rdar://problem/87269825>
Created attachment 448626 [details] Patch
Comment on attachment 448626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448626&action=review It'd be interesting to know if Simon agrees with this direction. > Source/WebCore/rendering/RenderLayer.cpp:1780 > +static inline bool isContainerForPositioned(RenderLayer& layer, PositionType position, bool establishesTopLayer) It's hard to tell what establishesTopLayer means in this context without looking at the callsites (like what establishes the top layer). > LayoutTests/TestExpectations:2361 > I am curious about this change (going from Skip -> Crash -while I understand that it is a more precise definition of the expected outcome, I am wondering if there are other reasons)
(In reply to zalan from comment #3) > > LayoutTests/TestExpectations:2361 > > > > I am curious about this change (going from Skip -> Crash -while I understand > that it is a more precise definition of the expected outcome, I am wondering > if there are other reasons) I changed it to Crash so if there is an unexpected PASS (either by this change, or by other changes I'm working on), it would be caught, while Skip would just leave it disabled, with less testing coverage.
Comment on attachment 448626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448626&action=review > Source/WebCore/rendering/RenderLayer.cpp:4549 > + auto parentLayer = establishesTopLayer() ? renderer().view().layer() : parent(); Calling this 'parentLayer' is confusing because it's not always the parent layer. Maybe containerLayer?
(In reply to zalan from comment #3) > Comment on attachment 448626 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=448626&action=review > > It'd be interesting to know if Simon agrees with this direction. > > > Source/WebCore/rendering/RenderLayer.cpp:1780 > > +static inline bool isContainerForPositioned(RenderLayer& layer, PositionType position, bool establishesTopLayer) > > It's hard to tell what establishesTopLayer means in this context without > looking at the callsites (like what establishes the top layer). I followed the naming of the `position` argument, though maybe we should look into merging both argument (taking a renderer?), but let's not do this here.
Created attachment 448773 [details] [fast-cq] Patch
Committed r287845 (245894@main): <https://commits.webkit.org/245894@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 448773 [details].