Bug 222952

Summary: Both the root and the <html> RenderLayers have scrollable areas
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Nikolas Zimmermann <zimmermann>
Status: NEW    
Severity: Normal CC: ahmad.saleem792, bfulgham, cdumez, changseok, esprehn+autocc, ews-watchlist, fred.wang, glenn, kondapallykalyan, lwarlow, mifenton, pdr, rbuis, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch rbuis: review?

Simon Fraser (smfr)
Reported 2021-03-08 16:02:03 PST
This is the layer tree for about:blank: layer 0x6edc61420 scrollableArea 0x6edca2780 at (0,0) size 980x876 (composited, bounds=at (0,0) size 980x876, drawsContent=1, paints into ancestor=0) RenderView 0x6f15492b0 at (0,0) size 980x876 positive z-order list (1) layer 0x6edc61528 scrollableArea 0x6edca2960 at (0,0) size 980x876 RenderBlock 0x6f1549760 {HTML} at (0,0) size 980x876 RenderBody 0x6f1549890 {BODY} at (8,8) size 964x860 Why are there scrollableAreas on the root and the <html>'s layer? The root should be covered by ScrollView, and the <html> should not independently have a scrollable area.
Attachments
Patch (1.46 KB, patch)
2021-12-17 10:36 PST, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (3.38 KB, patch)
2021-12-17 11:54 PST, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (1.41 KB, patch)
2022-03-31 01:38 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (2.67 KB, patch)
2022-03-31 09:34 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (3.03 KB, patch)
2022-04-02 10:03 PDT, Rob Buis 		rbuis: review?
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2021-03-08 16:45:51 PST
Maybe fix: diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp index 5dcebaabed94f77163a7f4b3db05cf1634aeecf4..8f441ac04b3162f5a56cac02807bd9a27e7d0fe7 100644 --- a/Source/WebCore/rendering/RenderBox.cpp +++ b/Source/WebCore/rendering/RenderBox.cpp @@ -959,8 +959,8 @@ bool RenderBox::isScrollableOrRubberbandableBox() const bool RenderBox::requiresLayerWithScrollableArea() const { // The RenderView is always expected to be potentially scrollable. - if (isRenderView() || isDocumentElementRenderer()) - return true; +// if (isRenderView() || isDocumentElementRenderer()) +// return true; // Overflow handling needs RenderLayerScrollableArea. if (scrollsOverflow() || hasOverflowClip() || hasHorizontalOverflow() || hasVerticalOverflow()) diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp index 1a1835a556a903c3b30fb99dd50025d3111b14ac..33aae0c34c2eabc833ade62a779bd7cb3e22c306 100644 --- a/Source/WebCore/rendering/RenderLayerCompositor.cpp +++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp @@ -4703,9 +4703,6 @@ LayoutRect RenderLayerCompositor::parentRelativeScrollableRect(const RenderLayer void RenderLayerCompositor::updateScrollingNodeLayers(ScrollingNodeID nodeID, RenderLayer& layer, ScrollingCoordinator& scrollingCoordinator) { - auto* scrollableArea = layer.scrollableArea(); - ASSERT(scrollableArea); - if (layer.isRenderViewLayer()) { FrameView& frameView = m_renderView.frameView(); scrollingCoordinator.setNodeLayers(nodeID, { nullptr, @@ -4713,6 +4710,8 @@ void RenderLayerCompositor::updateScrollingNodeLayers(ScrollingNodeID nodeID, Re fixedRootBackgroundLayer(), clipLayer(), rootContentsLayer(), frameView.layerForHorizontalScrollbar(), frameView.layerForVerticalScrollbar() }); } else { + auto* scrollableArea = layer.scrollableArea(); + ASSERT(scrollableArea); auto& backing = *layer.backing(); scrollingCoordinator.setNodeLayers(nodeID, { backing.graphicsLayer(), backing.scrollContainerLayer(), backing.scrolledContentsLayer(),
Radar WebKit Bug Importer
Comment 2 2021-03-15 17:02:15 PDT
<rdar://problem/75454829>
Rob Buis
Comment 3 2021-12-17 10:36:46 PST
Created attachment 447461 [details] Patch
Rob Buis
Comment 4 2021-12-17 11:54:56 PST
Created attachment 447465 [details] Patch
Simon Fraser (smfr)
Comment 5 2021-12-17 12:00:47 PST
Comment on attachment 447465 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447465&action=review > Source/WebCore/rendering/RenderBox.cpp:973 > + if (isRenderView()) > + return DeprecatedGlobalSettings::usesOverlayScrollbars(); Weird?
Rob Buis
Comment 6 2021-12-17 12:14:45 PST
Comment on attachment 447465 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447465&action=review >> Source/WebCore/rendering/RenderBox.cpp:973 >> + return DeprecatedGlobalSettings::usesOverlayScrollbars(); > > Weird? Kind of. It seems LayoutTests/fast/scrolling/overlay-scrollbars-scroll-corner.html relies on this setting to be enabled, but just setting it will not change the render tree, so I expect this test still will fail. Not sure how to propagate the setting yet...
Rob Buis
Comment 7 2022-03-31 01:38:40 PDT
Created attachment 456216 [details] Patch
Rob Buis
Comment 8 2022-03-31 09:34:44 PDT
Created attachment 456246 [details] Patch
Simon Fraser (smfr)
Comment 9 2022-03-31 10:50:51 PDT
Comment on attachment 456246 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=456246&action=review > Source/WebCore/rendering/RenderBox.cpp:992 > + if (isRenderView()) > + return DeprecatedGlobalSettings::usesOverlayScrollbars(); This is a bit odd. why is this necessary?
Rob Buis
Comment 10 2022-04-02 10:03:58 PDT
Created attachment 456460 [details] Patch
Simon Fraser (smfr)
Comment 11 2022-04-04 11:26:30 PDT
Comment on attachment 456460 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=456460&action=review > Source/WebCore/ChangeLog:11 > + The <html> element never requires a scrollable area. > + The root only requires it for the overlay scrollbar feature. > + When using setUsesOverlayScrollbars internals API make sure > + we enable the scrollable area on the layer. I'm a bit confused by this. Overlay scrollbars are the default state on macOS. Is this just something about how the testing API is hooked up?
Nikolas Zimmermann
Comment 12 2022-04-22 05:39:08 PDT
(In reply to Simon Fraser (smfr) from comment #11) > Comment on attachment 456460 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=456460&action=review > > > Source/WebCore/ChangeLog:11 > > + The <html> element never requires a scrollable area. > > + The root only requires it for the overlay scrollbar feature. > > + When using setUsesOverlayScrollbars internals API make sure > > + we enable the scrollable area on the layer. > > I'm a bit confused by this. Overlay scrollbars are the default state on > macOS. Is this just something about how the testing API is hooked up? Internals::resetToConsistentState() calls setUsesOverlayScrollbars(false), therefore during testing they are off-by-default, unless activated...
Rob Buis
Comment 13 2022-04-22 09:09:39 PDT
The updating code could go into DeprecreatedGlobalSettings::setUsesOverlayScrollbars if that is preferred.
Nikolas Zimmermann
Comment 14 2022-09-05 13:41:27 PDT
It would be great if the document element renderer wouldn't always need a RenderLayerScrollableArea. Rob, can we revisit the topic and fix it finally? Last status was: overlay scrollbars are on by default on macOS, but not during testing, unless activated by the specific layout test. Correct? Still your patch looked good I think, Simon only wanted to confirm that this affects only testing (they way it's hooked up). Right?
Note You need to log in before you can comment on or make changes to this bug.