[Services with UI] Action menu arrow hit testing is sometimes wrong.
authorenrica@apple.com <enrica@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Aug 2014 17:52:15 +0000 (17:52 +0000)
committerenrica@apple.com <enrica@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Aug 2014 17:52:15 +0000 (17:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135776
<rdar://problem/17837670>

Reviewed by Brady Eidson.

There was a problem in the algorithm that stitches together the selection rectangles
to be given to Data Detectors API.
This change adds a new function that stiches together all the rects contributing to the
first line, all the rects contributing to the last line and all the ones in the middle.
This way we can have a maximum of 3 non overlapping rectangles.

* WebProcess/WebPage/mac/ServicesOverlayController.mm:
(WebKit::stitchRects):
(WebKit::compactRectsWithGapRects):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@172395 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm

index c0ab96d..2503c8c 100644 (file)
@@ -1,3 +1,21 @@
+2014-08-08  Enrica Casucci  <enrica@apple.com>
+
+        [Services with UI] Action menu arrow hit testing is sometimes wrong.
+        https://bugs.webkit.org/show_bug.cgi?id=135776
+        <rdar://problem/17837670>
+
+        Reviewed by Brady Eidson.
+
+        There was a problem in the algorithm that stitches together the selection rectangles
+        to be given to Data Detectors API.
+        This change adds a new function that stiches together all the rects contributing to the
+        first line, all the rects contributing to the last line and all the ones in the middle.
+        This way we can have a maximum of 3 non overlapping rectangles.
+
+        * WebProcess/WebPage/mac/ServicesOverlayController.mm:
+        (WebKit::stitchRects):
+        (WebKit::compactRectsWithGapRects):
+
 2014-08-11  Gyuyoung Kim  <gyuyoung.kim@samsung.com>
 
         Unreviewed, EFL build fix since r172385.
index 14dbb05..4d8a8c0 100644 (file)
@@ -139,22 +139,59 @@ static void expandForGap(Vector<LayoutRect>& rects, uint8_t* alignments, const G
     }
 }
 
-static void compactRectsWithGapRects(Vector<LayoutRect>& rects, const Vector<GapRects>& gapRects)
+static inline void stitchRects(Vector<LayoutRect>& rects)
 {
-    if (rects.isEmpty())
+    if (rects.size() <= 1)
+        return;
+    
+    Vector<LayoutRect> newRects;
+    
+    // FIXME: Need to support vertical layout.
+    // First stitch together all the rects on the first line of the selection.
+    size_t indexFromStart = 0;
+    LayoutUnit firstTop = rects[indexFromStart].y();
+    LayoutRect& currentRect = rects[indexFromStart++];
+    while (indexFromStart < rects.size() && rects[indexFromStart].y() == firstTop)
+        currentRect.unite(rects[indexFromStart++]);
+    
+    newRects.append(currentRect);
+    if (indexFromStart == rects.size()) {
+        // All the rects are on one line. There is nothing else to do.
+        rects.swap(newRects);
+        return;
+    }
+    
+    // Next stitch together all the rects on the last line of the selection.
+    size_t indexFromEnd = rects.size() - 1;
+    LayoutUnit lastTop = rects[indexFromEnd].y();
+    LayoutRect lastRect = rects[indexFromEnd];
+    while (indexFromEnd != indexFromStart && rects[--indexFromEnd].y() == lastTop)
+        lastRect.unite(rects[indexFromEnd]);
+    
+    if (indexFromEnd == indexFromStart) {
+        // All the rects are on two lines only. There is nothing else to do.
+        newRects.append(lastRect);
+        rects.swap(newRects);
         return;
-
-    // All of the middle rects - everything but the first and last - can be unioned together.
-    if (rects.size() > 3) {
-        LayoutRect united;
-        for (unsigned i = 1; i < rects.size() - 1; ++i)
-            united.unite(rects[i]);
-
-        rects[1] = united;
-        rects[2] = rects.last();
-        rects.shrink(3);
     }
+    
+    // indexFromStart is the index of the first rectangle on the second line.
+    // indexFromEnd is the index of the last rectangle on the second to the last line.
+    // Stitch together all the rects after the first line until the second to the last included.
+    currentRect = rects[indexFromStart];
+    while (indexFromStart != indexFromEnd)
+        currentRect.unite(rects[++indexFromStart]);
+    
+    newRects.append(currentRect);
+    newRects.append(lastRect);
 
+    rects.swap(newRects);
+}
+
+static void compactRectsWithGapRects(Vector<LayoutRect>& rects, const Vector<GapRects>& gapRects)
+{
+    stitchRects(rects);
+    
     // FIXME: The following alignments are correct for LTR text.
     // We should also account for RTL.
     uint8_t alignments[3];