Drag snapshot does not paint for normal flow content #47192

nullhook · 2025-06-25T17:32:15Z

`f220997`

Drag snapshot does not paint for normal flow content
rdar://154202408

Reviewed by NOBODY (OOPS!).

Drag snapshots did not work for normal flow
content because enclosingLayer found the
nearest layer as the HTML root layer.
As a result, the layer used for painting
decisions was a much higher ancestor. We now
restrict this with a hasLayer check.

For non-layer roots, we exclude positioned
descendants from the drag image if they extend
outside the containing block.

All the special inclusion/exclusion
logic is now guraded behind a new paint behavior
DraggableSnapshot.

* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::snapshotNode):

Our snapshot tests only draggable
paint behavior.

f220997

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2025-06-25T17:32:22Z

EWS run on previous version of this PR (hash 269c1c7)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

LayoutTests/fast/snapshot/positioned-static-abs-expected.html

Source/WebCore/rendering/RenderLayer.cpp

lilyspiniolas · 2025-06-25T18:06:22Z

Source/WebCore/rendering/RenderLayer.cpp

+        } else {
+            if (renderer().isAbsolutelyPositioned() && &renderer() != paintingInfo.subtreePaintRoot) {


Suggested change

} else {

if (renderer().isAbsolutelyPositioned() && &renderer() != paintingInfo.subtreePaintRoot) {

} else if (renderer().isAbsolutelyPositioned() && &renderer() != paintingInfo.subtreePaintRoot) {

Source/WebCore/testing/Internals.cpp

webkit-early-warning-system · 2025-06-26T15:13:23Z

EWS run on previous version of this PR (hash 443f501)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

smfr

Please wrap the commit message to something sensible like 80 or 90 columns.

smfr · 2025-06-30T15:15:51Z

Source/WebCore/platform/DragImage.cpp

@@ -120,7 +120,11 @@ static DragImageRef createDragImageFromSnapshot(RefPtr<ImageBuffer> snapshot, No
 DragImageRef createDragImageForNode(LocalFrame& frame, Node& node)
 {
    ScopedNodeDragEnabler enableDrag(frame, node);
-    return createDragImageFromSnapshot(snapshotNode(frame, node, { { }, ImageBufferPixelFormat::BGRA8, DestinationColorSpace::SRGB() }), &node);
+
+    SnapshotOptions options { { }, ImageBufferPixelFormat::BGRA8, DestinationColorSpace::SRGB() };


Consider:

auto options = SnapshotOptions options { { SnapshotFlags::DraggableElement }, ImageBufferPixelFormat::BGRA8, DestinationColorSpace::SRGB() }; { { }, ImageBufferPixelFormat::BGRA8, DestinationColorSpace::SRGB() };

smfr · 2025-06-30T15:16:00Z

Source/WebCore/platform/DragImage.cpp

@@ -218,7 +222,10 @@ DragImageRef createDragImageForImage(LocalFrame& frame, Node& node, IntRect& ima
    elementRect = snappedIntRect(topLevelRect);
    imageRect = paintingRect;

-    return createDragImageFromSnapshot(snapshotNode(frame, node, { { }, ImageBufferPixelFormat::BGRA8, DestinationColorSpace::SRGB() }), &node);
+    SnapshotOptions options { { }, ImageBufferPixelFormat::BGRA8, DestinationColorSpace::SRGB() };


smfr · 2025-06-30T15:17:08Z

Source/WebCore/rendering/RenderLayer.cpp

            bool isLayerInSubtree = (this == subtreeRootLayer) || isDescendantOf(*subtreeRootLayer);

            if (isLayerInSubtree) {
-                if (paintingInfo.subtreePaintRoot != &renderer()) {
+                if (&renderer() != paintingInfo.subtreePaintRoot) {


No need to flip this.

smfr · 2025-06-30T15:20:05Z

Source/WebCore/rendering/RenderLayer.cpp

                    if (CheckedPtr rootAsBlock = dynamicDowncast<RenderBlock>(paintingInfo.subtreePaintRoot)) {
                        if (!rootAsBlock->isContainingBlockAncestorFor(renderer()))
                            shouldPaintContent = false;
                    }
                }
            } else
                shouldPaintContent = false;
+        } else {
+            if (renderer().isAbsolutelyPositioned() && &renderer() != paintingInfo.subtreePaintRoot) {
+                if (CheckedPtr rootAsBlock = dynamicDowncast<RenderBlock>(paintingInfo.subtreePaintRoot)) {


I think this branch also behaves correctly for the case where paintingInfo.subtreePaintRoot->hasLayer() is true, so consider removing the first branch.

If you do keep both, factor the isContainingBlockAncestorFor chunk into a lambda to avoid repetition.

smfr · 2025-06-30T15:20:44Z

Source/WebCore/testing/Internals.cpp

@@ -2119,6 +2119,7 @@ ExceptionOr<RefPtr<ImageData>> Internals::snapshotNode(Node& node)
        return Exception { ExceptionCode::InvalidAccessError };

    SnapshotOptions options { { }, ImageBufferPixelFormat::BGRA8, DestinationColorSpace::SRGB() };
+    options.flags.add(SnapshotFlags::DraggableElement);


smfr · 2025-06-30T15:21:15Z

LayoutTests/fast/snapshot/positioned-static.html

+}
+
+window.addEventListener('load', () => {
+    setTimeout(main, 200);


Don't use a 200ms timeout. If every one of our thousands of layout tests took 200ms, they would take hours to run.

smfr · 2025-06-30T15:21:40Z

LayoutTests/fast/snapshot/positioned-static.html

+        return;
+    }
+
+    const imageData = internals.snapshotNode(box);


Consider moving this shared JS into a resources/snapshot-helper.js file.

rdar://154202408 Reviewed by NOBODY (OOPS!). Drag snapshots did not work for normal flow content because enclosingLayer found the nearest layer as the HTML root layer. As a result, the layer used for painting decisions was a much higher ancestor. We now restrict this with a hasLayer check. For non-layer roots, we exclude positioned descendants from the drag image if they extend outside the containing block. All the special inclusion/exclusion logic is now guraded behind a new paint behavior DraggableSnapshot. * Source/WebCore/testing/Internals.cpp: (WebCore::Internals::snapshotNode): Our snapshot tests only draggable paint behavior.

webkit-early-warning-system · 2025-07-01T00:11:38Z

EWS run on current version of this PR (hash f220997)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

nullhook requested a review from cdumez as a code owner June 25, 2025 17:32

nullhook self-assigned this Jun 25, 2025

nullhook requested a review from smfr June 25, 2025 17:33

lilyspiniolas reviewed Jun 25, 2025

View reviewed changes

LayoutTests/fast/snapshot/positioned-static-abs-expected.html Outdated Show resolved Hide resolved

lilyspiniolas reviewed Jun 25, 2025

View reviewed changes

Source/WebCore/rendering/RenderLayer.cpp Outdated Show resolved Hide resolved

lilyspiniolas reviewed Jun 25, 2025

View reviewed changes

Source/WebCore/testing/Internals.cpp Outdated Show resolved Hide resolved

nullhook force-pushed the eng/154202408 branch from 269c1c7 to 443f501 Compare June 26, 2025 15:13

smfr reviewed Jun 30, 2025

View reviewed changes

nullhook force-pushed the eng/154202408 branch from 443f501 to f220997 Compare July 1, 2025 00:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Drag snapshot does not paint for normal flow content #47192

Drag snapshot does not paint for normal flow content #47192

nullhook commented Jun 25, 2025 •

edited by webkit-early-warning-system

Loading

Uh oh!

webkit-early-warning-system commented Jun 25, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

lilyspiniolas Jun 25, 2025

Uh oh!

Uh oh!

webkit-early-warning-system commented Jun 26, 2025 •

edited

Loading

Uh oh!

smfr left a comment

Uh oh!

smfr Jun 30, 2025

Uh oh!

smfr Jun 30, 2025

Uh oh!

smfr Jun 30, 2025

Uh oh!

smfr Jun 30, 2025

Uh oh!

smfr Jun 30, 2025

Uh oh!

smfr Jun 30, 2025

Uh oh!

smfr Jun 30, 2025

Uh oh!

webkit-early-warning-system commented Jul 1, 2025 •

edited

Loading

Uh oh!

Uh oh!

		} else {
		if (renderer().isAbsolutelyPositioned() && &renderer() != paintingInfo.subtreePaintRoot) {

	} else {
	if (renderer().isAbsolutelyPositioned() && &renderer() != paintingInfo.subtreePaintRoot) {
	} else if (renderer().isAbsolutelyPositioned() && &renderer() != paintingInfo.subtreePaintRoot) {

Drag snapshot does not paint for normal flow content #47192

Are you sure you want to change the base?

Drag snapshot does not paint for normal flow content #47192

Conversation

nullhook commented Jun 25, 2025 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

webkit-early-warning-system commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smfr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nullhook commented Jun 25, 2025 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Jun 25, 2025 •

edited

Loading

webkit-early-warning-system commented Jun 26, 2025 •

edited

Loading

webkit-early-warning-system commented Jul 1, 2025 •

edited

Loading