222952 – Both the root and the <html> RenderLayers have scrollable areas

NEW 222952

Both the root and the <html> RenderLayers have scrollable areas

https://bugs.webkit.org/show_bug.cgi?id=222952

Summary Both the root and the <html> RenderLayers have scrollable areas

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	Details Formatted Diff Diff
Patch (3.38 KB, patch) 2021-12-17 11:54 PST, Rob Buis	no flags	Details Formatted Diff Diff
Patch (1.41 KB, patch) 2022-03-31 01:38 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Patch (2.67 KB, patch) 2022-03-31 09:34 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Patch (3.03 KB, patch) 2022-04-02 10:03 PDT, Rob Buis	rbuis: review?	Details Formatted Diff Diff
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.

Status NEW

Resolution

Priority P2

Severity Normal

Classification Unclassified

Version Safari Technology Preview

Hardware Unspecified

OS Unspecified

Product WebKit

Component Layout and Rendering

Assignee

Nikolas Zimmermann

Reported

2021-03-08 16:02 PST

Modified

2023-07-08 16:02 PDT History

CC List

16 users Show

URL

Keywords InRadar

Depends on

Blocks