InteractionRegion for wrapped text has multiple rects instead of one
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 May 2022 19:26:27 +0000 (19:26 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 May 2022 19:26:27 +0000 (19:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=240748

Reviewed by Wenson Hsieh.

* Source/WebCore/page/InteractionRegion.h:
(WebCore::operator==):
(WebCore::InteractionRegion::encode const):
(WebCore::InteractionRegion::decode):
* Source/WebCore/rendering/EventRegion.cpp:
(WebCore::EventRegion::translate):
(WebCore::EventRegion::dump const):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeInteractionRegionLayers.mm:
(WebKit::updateLayersForInteractionRegions):
* Source/WebCore/page/DebugPageOverlays.cpp:
(WebCore::pathsForRegion):
(WebCore::InteractionRegionOverlay::activeRegion):
(WebCore::InteractionRegionOverlay::drawRect):
* Source/WebCore/page/InteractionRegion.cpp:
(WebCore::regionForElement):
Maintain InteractionRegion geometry as a Region instead of a vector of rects,
so that we can smush overlapping rectangles together on a per-InteractionRegion basis.

(WebCore::operator<<):
Add dumping code for InteractionRegion (and adopt it in EventRegion).

* LayoutTests/TestExpectations:
* LayoutTests/interaction-region/click-handler-expected.txt: Added.
* LayoutTests/interaction-region/click-handler.html: Added.
* LayoutTests/interaction-region/inline-link-dark-background-expected.txt: Added.
* LayoutTests/interaction-region/inline-link-dark-background.html: Added.
* LayoutTests/interaction-region/inline-link-expected.txt: Added.
* LayoutTests/interaction-region/inline-link.html: Added.
* LayoutTests/interaction-region/split-inline-link-expected.txt: Added.
* LayoutTests/interaction-region/split-inline-link.html: Added.
* LayoutTests/interaction-region/wrapped-inline-link-expected.txt: Added.
* LayoutTests/interaction-region/wrapped-inline-link.html: Added.
Add some basic interaction region tests, currently disabled by default
because they can only be run if you turn on the build-time flag.

"wrapped-inline-link.html" covers the case we are fixing by adopting Region;
the others are more generic tests that we should have had before.

link: https://commits.webkit.org/250840@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294614 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
LayoutTests/TestExpectations
LayoutTests/interaction-region/click-handler-expected.txt [new file with mode: 0644]
LayoutTests/interaction-region/click-handler.html [new file with mode: 0644]
LayoutTests/interaction-region/inline-link-dark-background-expected.txt [new file with mode: 0644]
LayoutTests/interaction-region/inline-link-dark-background.html [new file with mode: 0644]
LayoutTests/interaction-region/inline-link-expected.txt [new file with mode: 0644]
LayoutTests/interaction-region/inline-link.html [new file with mode: 0644]
LayoutTests/interaction-region/split-inline-link-expected.txt [new file with mode: 0644]
LayoutTests/interaction-region/split-inline-link.html [new file with mode: 0644]
LayoutTests/interaction-region/wrapped-inline-link-expected.txt [new file with mode: 0644]
LayoutTests/interaction-region/wrapped-inline-link.html [new file with mode: 0644]
Source/WebCore/page/DebugPageOverlays.cpp
Source/WebCore/page/InteractionRegion.cpp
Source/WebCore/page/InteractionRegion.h
Source/WebCore/rendering/EventRegion.cpp
Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeInteractionRegionLayers.mm

index 6de98d2c6884951aed501c07660e4ea232d142ae..37af21c7246fa12c7588294e48e776e033ded426 100644 (file)
@@ -88,6 +88,7 @@ fast/media/ios [ Skip ]
 fast/dom/Range/mac [ Skip ]
 remote-layer-tree/ios [ Skip ]
 inspector/page/setScreenSizeOverride.html [ Skip ]
+interaction-region [ Skip ]
 
 # Requires async overflow scrolling
 compositing/shared-backing/overflow-scroll [ Skip ]
diff --git a/LayoutTests/interaction-region/click-handler-expected.txt b/LayoutTests/interaction-region/click-handler-expected.txt
new file mode 100644 (file)
index 0000000..68d1799
--- /dev/null
@@ -0,0 +1,23 @@
+ (GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #FFFFFF)
+      (event region
+        (rect (0,0) width=800 height=600)
+
+      (interaction regions [
+        (region
+            (rect (0,0) width=100 height=100)
+)
+        (hasLightBackground 1)
+        (borderRadius 10.00)])
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/interaction-region/click-handler.html b/LayoutTests/interaction-region/click-handler.html
new file mode 100644 (file)
index 0000000..3f14645
--- /dev/null
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<style>
+    body { margin: 0; }
+
+    #button {
+        display: inline-block;
+        width: 100px;
+        height: 100px;
+        background-color: green;
+        cursor: pointer;
+        border-radius: 10px;
+    }
+</style>
+<body>
+<div id="button" onclick="click()"></div>
+
+<pre id="results"></pre>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+window.onload = function () {
+    if (window.internals)
+       results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+};
+</script>
+</body>
+</html>
diff --git a/LayoutTests/interaction-region/inline-link-dark-background-expected.txt b/LayoutTests/interaction-region/inline-link-dark-background-expected.txt
new file mode 100644 (file)
index 0000000..a82d8f3
--- /dev/null
@@ -0,0 +1,24 @@
+This is a link.
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #000000)
+      (event region
+        (rect (0,0) width=800 height=600)
+
+      (interaction regions [
+        (region
+            (rect (-3,-3) width=35 height=24)
+)
+        (hasLightBackground 0)
+        (borderRadius 0.00)])
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/interaction-region/inline-link-dark-background.html b/LayoutTests/interaction-region/inline-link-dark-background.html
new file mode 100644 (file)
index 0000000..1d55461
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<style>
+    body {
+        margin: 0;
+        background-color: black;
+        color: white;
+    }
+</style>
+<body>
+<a href="#">This</a> is a link.
+
+<pre id="results"></pre>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+window.onload = function () {
+    if (window.internals)
+       results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+};
+</script>
+</body>
+</html>
diff --git a/LayoutTests/interaction-region/inline-link-expected.txt b/LayoutTests/interaction-region/inline-link-expected.txt
new file mode 100644 (file)
index 0000000..0c9a801
--- /dev/null
@@ -0,0 +1,24 @@
+This is a link.
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #FFFFFF)
+      (event region
+        (rect (0,0) width=800 height=600)
+
+      (interaction regions [
+        (region
+            (rect (-3,-3) width=35 height=24)
+)
+        (hasLightBackground 1)
+        (borderRadius 0.00)])
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/interaction-region/inline-link.html b/LayoutTests/interaction-region/inline-link.html
new file mode 100644 (file)
index 0000000..607b91a
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<style>
+    body { margin: 0; }
+</style>
+<body>
+<a href="#">This</a> is a link.
+
+<pre id="results"></pre>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+window.onload = function () {
+    if (window.internals)
+       results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+};
+</script>
+</body>
+</html>
diff --git a/LayoutTests/interaction-region/split-inline-link-expected.txt b/LayoutTests/interaction-region/split-inline-link-expected.txt
new file mode 100644 (file)
index 0000000..b9bf73b
--- /dev/null
@@ -0,0 +1,28 @@
+A bunch of text before the first line
+short second line.
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #FFFFFF)
+      (event region
+        (rect (0,0) width=800 height=600)
+
+      (interaction regions [
+        (region
+            (rect (197,-3) width=31 height=18)
+            (rect (-3,15) width=115 height=6)
+            (rect (197,15) width=31 height=6)
+            (rect (-3,21) width=115 height=18)
+)
+        (hasLightBackground 1)
+        (borderRadius 0.00)])
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/interaction-region/split-inline-link.html b/LayoutTests/interaction-region/split-inline-link.html
new file mode 100644 (file)
index 0000000..f3c7d3e
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<style>
+    body { margin: 0; }
+</style>
+<body>
+A bunch of text before the first <a href="#">line<br/>short second line</a>.
+
+<pre id="results"></pre>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+window.onload = function () {
+    if (window.internals)
+       results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+};
+</script>
+</body>
+</html>
diff --git a/LayoutTests/interaction-region/wrapped-inline-link-expected.txt b/LayoutTests/interaction-region/wrapped-inline-link-expected.txt
new file mode 100644 (file)
index 0000000..2f688fd
--- /dev/null
@@ -0,0 +1,27 @@
+Line
+Line
+Line
+Line
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #FFFFFF)
+      (event region
+        (rect (0,0) width=800 height=600)
+
+      (interaction regions [
+        (region
+            (rect (-3,-3) width=36 height=78)
+)
+        (hasLightBackground 1)
+        (borderRadius 0.00)])
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/interaction-region/wrapped-inline-link.html b/LayoutTests/interaction-region/wrapped-inline-link.html
new file mode 100644 (file)
index 0000000..d7a4982
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<style>
+    body { margin: 0; }
+</style>
+<body>
+<a href="#">Line<br/>Line<br/>Line<br/>Line</a>
+
+<pre id="results"></pre>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+window.onload = function () {
+    if (window.internals)
+       results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+};
+</script>
+</body>
+</html>
index 19eede4c995a30a1caa273bec5de3b849534c9a9..115ac4442a9ec60be028671205c310e29cdbe8c3 100644 (file)
@@ -295,9 +295,9 @@ static Vector<Path> pathsForRegion(const InteractionRegion& region)
 {
     static constexpr float radius = 4;
 
-    Vector<FloatRect> rects;
-    for (auto rect : region.rectsInContentCoordinates)
-        rects.append(rect);
+    Vector<FloatRect> rects = region.regionInLayerCoordinates.rects().map([] (auto rect) -> FloatRect {
+        return rect;
+    });
     return PathUtilities::pathsWithShrinkWrappedRects(rects, std::max(region.borderRadius, radius));
 }
 
@@ -309,7 +309,7 @@ std::optional<InteractionRegion> InteractionRegionOverlay::activeRegion()
     for (const auto& region : m_regions) {
         float area = 0;
         FloatRect boundingRect;
-        for (const auto& rect : region.rectsInContentCoordinates) {
+        for (const auto& rect : region.regionInLayerCoordinates.rects()) {
             if (boundingRect.isEmpty())
                 boundingRect = rect;
             else
@@ -433,8 +433,8 @@ void InteractionRegionOverlay::drawRect(PageOverlay&, GraphicsContext& context,
         context.setStrokeColor(Color::green);
 
         for (const auto& region : m_regions) {
-            for (const auto& rect : region.rectsInContentCoordinates)
-            context.strokeRect(rect, 2);
+            for (const auto& rect : region.regionInLayerCoordinates.rects())
+                context.strokeRect(rect, 2);
         }
     }
 
index 1b2be6a2612e467e86a8a7fe447f8d882bc7bfef..8469350328f76fcb677854ef2fc1db1e495400ed 100644 (file)
@@ -111,20 +111,20 @@ static std::optional<InteractionRegion> regionForElement(Element& element)
         RoundedRect::Radii borderRadii = downcast<RenderBox>(*renderer).borderRadii();
         region.borderRadius = borderRadii.minimumRadius();
     }
-
-    region.rectsInContentCoordinates = compactMap(rectsInContentCoordinates, [&](auto rect) -> std::optional<FloatRect> {
+    
+    for (auto rect : rectsInContentCoordinates) {
         auto contentsRect = rect;
 
         if (&frameView != &mainFrameView)
             contentsRect.intersect(frameClipRect);
 
         if (contentsRect.isEmpty())
-            return std::nullopt;
+            continue;
 
-        return contentsRect;
-    });
+        region.regionInLayerCoordinates.unite(enclosingIntRect(contentsRect));
+    }
 
-    if (region.rectsInContentCoordinates.isEmpty())
+    if (region.regionInLayerCoordinates.isEmpty())
         return std::nullopt;
 
     return region;
@@ -186,4 +186,13 @@ Vector<InteractionRegion> interactionRegions(Page& page, FloatRect rect)
     return regions;
 }
 
+TextStream& operator<<(TextStream& ts, const InteractionRegion& interactionRegion)
+{
+    ts.dumpProperty("region", interactionRegion.regionInLayerCoordinates);
+    ts.dumpProperty("hasLightBackground", interactionRegion.hasLightBackground);
+    ts.dumpProperty("borderRadius", interactionRegion.borderRadius);
+
+    return ts;
+}
+
 }
index 37a57315a8cecdfcc4a679b4499ef63ce091bad7..e1f4fd8c32fbc4473e50e3b1fa16f18c482971e4 100644 (file)
 #pragma once
 
 #include "FloatRect.h"
+#include "Region.h"
 
 namespace IPC {
 class Decoder;
 class Encoder;
 }
 
+namespace WTF {
+class TextStream;
+}
+
 namespace WebCore {
 
 class Page;
 
 struct InteractionRegion {
-    Vector<FloatRect> rectsInContentCoordinates;
+    Region regionInLayerCoordinates;
     bool hasLightBackground { false };
     float borderRadius { 0 };
-    
+
     WEBCORE_EXPORT ~InteractionRegion();
-    
+
     template<class Encoder> void encode(Encoder&) const;
     template<class Decoder> static std::optional<InteractionRegion> decode(Decoder&);
 };
 
 inline bool operator==(const InteractionRegion& a, const InteractionRegion& b)
 {
-    return a.rectsInContentCoordinates == b.rectsInContentCoordinates
+    return a.regionInLayerCoordinates == b.regionInLayerCoordinates
         && a.hasLightBackground == b.hasLightBackground
         && a.borderRadius == b.borderRadius;
 }
 
 WEBCORE_EXPORT Vector<InteractionRegion> interactionRegions(Page&, FloatRect rectInContentCoordinates);
 
+WTF::TextStream& operator<<(WTF::TextStream&, const InteractionRegion&);
+
 template<class Encoder>
 void InteractionRegion::encode(Encoder& encoder) const
 {
-    encoder << rectsInContentCoordinates;
+    encoder << regionInLayerCoordinates;
     encoder << hasLightBackground;
     encoder << borderRadius;
 }
@@ -67,9 +74,9 @@ void InteractionRegion::encode(Encoder& encoder) const
 template<class Decoder>
 std::optional<InteractionRegion> InteractionRegion::decode(Decoder& decoder)
 {
-    std::optional<Vector<FloatRect>> rectsInContentCoordinates;
-    decoder >> rectsInContentCoordinates;
-    if (!rectsInContentCoordinates)
+    std::optional<Region> regionInLayerCoordinates;
+    decoder >> regionInLayerCoordinates;
+    if (!regionInLayerCoordinates)
         return std::nullopt;
     
     std::optional<bool> hasLightBackground;
@@ -83,7 +90,7 @@ std::optional<InteractionRegion> InteractionRegion::decode(Decoder& decoder)
         return std::nullopt;
 
     return { {
-        WTFMove(*rectsInContentCoordinates),
+        WTFMove(*regionInLayerCoordinates),
         WTFMove(*hasLightBackground),
         WTFMove(*borderRadius)
     } };
index 5575508509c24edff91e20d6665e364eb4846cc5..7b41aaae428baecc4e9b27bae5529db35d1e7109 100644 (file)
@@ -171,10 +171,8 @@ void EventRegion::translate(const IntSize& offset)
 #endif
 
 #if ENABLE(INTERACTION_REGIONS_IN_EVENT_REGION)
-    for (auto& region : m_interactionRegions) {
-        for (auto& rect : region.rectsInContentCoordinates)
-            rect.move(offset);
-    }
+    for (auto& region : m_interactionRegions)
+        region.regionInLayerCoordinates.translate(offset);
 #endif
 }
 
@@ -369,6 +367,13 @@ void EventRegion::dump(TextStream& ts) const
         ts << indent << ")\n";
     }
 #endif
+    
+#if ENABLE(INTERACTION_REGIONS_IN_EVENT_REGION)
+    if (!m_interactionRegions.isEmpty()) {
+        ts.dumpProperty("interaction regions", m_interactionRegions);
+        ts << "\n";
+    }
+#endif
 }
 
 TextStream& operator<<(TextStream& ts, TouchAction touchAction)
index b81096938f9afcb722824cbb5ff0bdc89ba069a5..76cb396ac2fae3a1dbd1f444ce60b046856be292 100644 (file)
@@ -98,8 +98,8 @@ void updateLayersForInteractionRegions(CALayer *layer, const RemoteLayerTreeTran
     bool applyBackgroundColorForDebugging = [[NSUserDefaults standardUserDefaults] boolForKey:@"WKInteractionRegionDebugFill"];
 
     for (const WebCore::InteractionRegion& region : properties.eventRegion.interactionRegions()) {
-        for (FloatRect rect : region.rectsInContentCoordinates) {
-            auto layerIterator = interactionRegionLayers.find(enclosingIntRect(rect));
+        for (IntRect rect : region.regionInLayerCoordinates.rects()) {
+            auto layerIterator = interactionRegionLayers.find(rect);
 
             RetainPtr<CALayer> interactionRegionLayer;
             if (layerIterator != interactionRegionLayers.end()) {