Selection rects sent to ServicesOverlayController are wrong.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 Jul 2014 17:56:38 +0000 (17:56 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 Jul 2014 17:56:38 +0000 (17:56 +0000)
<rdar://problem/16727796> and https://bugs.webkit.org/show_bug.cgi?id=134568

Reviewed by Darin Adler (and Tim Horton and Ryosuke Niwa).

Source/WebCore:
* WebCore.exp.in:

Update the gatherer to keep GapRects separate from LayoutRects:
* editing/SelectionRectGatherer.cpp:
(WebCore::SelectionRectGatherer::addRects):
(WebCore::SelectionRectGatherer::Notifier::~Notifier):
(WebCore::SelectionRectGatherer::clearAndCreateNotifier):
* editing/SelectionRectGatherer.h:

* page/EditorClient.h:
(WebCore::EditorClient::selectionRectsDidChange): Updated to take LayoutRects and GapRects separately.

Change RenderSelectionInfo to also hang on to the individual rects that formed the final bounding rect:
* rendering/RenderSelectionInfo.h:
(WebCore::RenderSelectionInfo::RenderSelectionInfo): If the RenderObject is a RenderText, then call
  collectSelectionRectsForLineBoxes instead of selectionRectForRepaint.
(WebCore::RenderSelectionInfo::rects):

* rendering/RenderText.cpp:
(WebCore::RenderText::collectSelectionRectsForLineBoxes): Added
(WebCore::RenderText::selectionRectForRepaint):
* rendering/RenderText.h:

* rendering/RenderTextLineBoxes.cpp:
(WebCore::RenderTextLineBoxes::collectSelectionRectsForRange): Added
* rendering/RenderTextLineBoxes.h:

* rendering/RenderView.cpp:
(WebCore::RenderView::setSubtreeSelection): Add the list of rects to the gatherer instead of just
  the bounding rect.

Source/WebKit2:
* WebProcess/WebCoreSupport/WebEditorClient.cpp:
(WebKit::WebEditorClient::selectionRectsDidChange): Also pass the GapRects to the ServicesOverlayController.
* WebProcess/WebCoreSupport/WebEditorClient.h:

* WebProcess/WebPage/ServicesOverlayController.h:
* WebProcess/WebPage/mac/ServicesOverlayController.mm:
(WebKit::expandForGap):
(WebKit::compactRectsWithGapRects): Combine 3+ rects down to exactly 3 rects, then expand them based on GapRects.
(WebKit::ServicesOverlayController::selectionRectsDidChange): Call compactRectsWithGapRects, then reverse the list.
(WebKit::ServicesOverlayController::drawSelectionHighlight): Tell data detectors to flip this.
(WebKit::ServicesOverlayController::drawTelephoneNumberHighlight): Tell data detectors to flip this.
(WebKit::ServicesOverlayController::drawCurrentHighlight): No need to flip this anymore.

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

16 files changed:
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/editing/SelectionRectGatherer.cpp
Source/WebCore/editing/SelectionRectGatherer.h
Source/WebCore/page/EditorClient.h
Source/WebCore/rendering/RenderSelectionInfo.h
Source/WebCore/rendering/RenderText.cpp
Source/WebCore/rendering/RenderText.h
Source/WebCore/rendering/RenderTextLineBoxes.cpp
Source/WebCore/rendering/RenderTextLineBoxes.h
Source/WebCore/rendering/RenderView.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp
Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h
Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h
Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm

index 40b7019..dab4bc5 100644 (file)
 
 2014-07-03  Brady Eidson  <beidson@apple.com>
 
+        Selection rects sent to ServicesOverlayController are wrong.
+        <rdar://problem/16727796> and https://bugs.webkit.org/show_bug.cgi?id=134568
+
+        Reviewed by Darin Adler (and Tim Horton and Ryosuke Niwa).
+
+        * WebCore.exp.in:
+
+        Update the gatherer to keep GapRects separate from LayoutRects:
+        * editing/SelectionRectGatherer.cpp:
+        (WebCore::SelectionRectGatherer::addRects):
+        (WebCore::SelectionRectGatherer::Notifier::~Notifier):
+        (WebCore::SelectionRectGatherer::clearAndCreateNotifier):
+        * editing/SelectionRectGatherer.h:
+
+        * page/EditorClient.h:
+        (WebCore::EditorClient::selectionRectsDidChange): Updated to take LayoutRects and GapRects separately.
+
+        Change RenderSelectionInfo to also hang on to the individual rects that formed the final bounding rect:
+        * rendering/RenderSelectionInfo.h:
+        (WebCore::RenderSelectionInfo::RenderSelectionInfo): If the RenderObject is a RenderText, then call
+          collectSelectionRectsForLineBoxes instead of selectionRectForRepaint.
+        (WebCore::RenderSelectionInfo::rects):
+
+        * rendering/RenderText.cpp:
+        (WebCore::RenderText::collectSelectionRectsForLineBoxes): Added
+        (WebCore::RenderText::selectionRectForRepaint):
+        * rendering/RenderText.h:
+
+        * rendering/RenderTextLineBoxes.cpp:
+        (WebCore::RenderTextLineBoxes::collectSelectionRectsForRange): Added
+        * rendering/RenderTextLineBoxes.h:
+
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::setSubtreeSelection): Add the list of rects to the gatherer instead of just 
+          the bounding rect.
+
+2014-07-03  Brady Eidson  <beidson@apple.com>
+
         Possible crash in IconDatabase in WebCore::IconDatabase::dispatchDidRemoveAllIconsOnMainThread
         <rdar://problem/17437687> and https://bugs.webkit.org/show_bug.cgi?id=134517
 
index eee480f..4bfb801 100644 (file)
@@ -78,6 +78,7 @@ __ZN7WebCore10FloatPointC1ERKNS_8IntPointE
 __ZN7WebCore10FontGlyphs15releaseFontDataEv
 __ZN7WebCore10JSDocument6s_infoE
 __ZN7WebCore10LayoutRect5scaleEf
+__ZN7WebCore10LayoutRect5uniteERKS0_
 __ZN7WebCore10LayoutRectC1ERKNS_9FloatRectE
 __ZN7WebCore10MouseEvent6createERKN3WTF12AtomicStringENS1_10PassRefPtrINS_9DOMWindowEEERKNS_18PlatformMouseEventEiNS5_INS_4NodeEEE
 __ZN7WebCore10Pasteboard14writePlainTextERKN3WTF6StringENS0_18SmartReplaceOptionE
index c4886da..5a7ddb8 100644 (file)
@@ -48,12 +48,7 @@ void SelectionRectGatherer::addRect(const LayoutRect& rect)
 
 void SelectionRectGatherer::addRects(const GapRects& rects)
 {
-    if (!rects.left().isEmpty())
-        m_rects.append(rects.left());
-    if (!rects.right().isEmpty())
-        m_rects.append(rects.right());
-    if (!rects.center().isEmpty())
-        m_rects.append(rects.center());
+    m_gapRects.append(rects);
 }
 
 SelectionRectGatherer::Notifier::Notifier(SelectionRectGatherer& gatherer)
@@ -64,12 +59,13 @@ SelectionRectGatherer::Notifier::Notifier(SelectionRectGatherer& gatherer)
 SelectionRectGatherer::Notifier::~Notifier()
 {
     if (EditorClient* client = m_gatherer.m_renderView.view().frame().editor().client())
-        client->selectionRectsDidChange(m_gatherer.m_rects);
+        client->selectionRectsDidChange(m_gatherer.m_rects, m_gatherer.m_gapRects);
 }
 
 std::unique_ptr<SelectionRectGatherer::Notifier> SelectionRectGatherer::clearAndCreateNotifier()
 {
     m_rects.clear();
+    m_gapRects.clear();
 
     return std::make_unique<Notifier>(*this);
 }
index 90857d5..f87e52f 100644 (file)
@@ -62,6 +62,7 @@ public:
 private:
     RenderView& m_renderView;
     Vector<LayoutRect> m_rects;
+    Vector<GapRects> m_gapRects;
 };
 
 } // namespace WebCore
index 1159621..8f4d3cf 100644 (file)
@@ -64,6 +64,7 @@ class TextCheckerClient;
 class VisibleSelection;
 class VisiblePosition;
 
+struct GapRects;
 struct GrammarDetail;
 
 class EditorClient {
@@ -181,7 +182,7 @@ public:
 
 #if ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)
     virtual void selectedTelephoneNumberRangesChanged(const Vector<RefPtr<Range>>&) { }
-    virtual void selectionRectsDidChange(const Vector<LayoutRect>&) { }
+    virtual void selectionRectsDidChange(const Vector<LayoutRect>&, const Vector<GapRects>&) { }
 #endif
 
     // Support for global selections, used on platforms like the X Window System that treat
index 67365c6..46e886f 100644 (file)
@@ -27,6 +27,7 @@
 
 #include "IntRect.h"
 #include "RenderBox.h"
+#include "RenderText.h"
 
 namespace WebCore {
 
@@ -62,8 +63,14 @@ class RenderSelectionInfo : public RenderSelectionInfoBase {
 public:
     RenderSelectionInfo(RenderObject* o, bool clipToVisibleContent)
         : RenderSelectionInfoBase(o)
-        , m_rect(o->canUpdateSelectionOnRootLineBoxes() ? o->selectionRectForRepaint(m_repaintContainer, clipToVisibleContent) : LayoutRect())
     {
+        ASSERT(o);
+        if (o->canUpdateSelectionOnRootLineBoxes()) {
+            if (o->isText())
+                m_rect = toRenderText(*o).collectSelectionRectsForLineBoxes(m_repaintContainer, clipToVisibleContent, m_rects);
+            else
+                m_rect = o->selectionRectForRepaint(m_repaintContainer, clipToVisibleContent);
+        }
     }
     
     void repaint()
@@ -71,9 +78,11 @@ public:
         m_object->repaintUsingContainer(m_repaintContainer, enclosingIntRect(m_rect));
     }
 
+    const Vector<LayoutRect>& rects() const { return m_rects; }
     LayoutRect rect() const { return m_rect; }
 
 private:
+    Vector<LayoutRect> m_rects;
     LayoutRect m_rect; // relative to repaint container
 };
 
index b58716c..409ad6b 100644 (file)
@@ -1237,7 +1237,7 @@ LayoutRect RenderText::clippedOverflowRectForRepaint(const RenderLayerModelObjec
     return rendererToRepaint->clippedOverflowRectForRepaint(repaintContainer);
 }
 
-LayoutRect RenderText::selectionRectForRepaint(const RenderLayerModelObject* repaintContainer, bool clipToVisibleContent)
+LayoutRect RenderText::collectSelectionRectsForLineBoxes(const RenderLayerModelObject* repaintContainer, bool clipToVisibleContent, Vector<LayoutRect>* rects)
 {
     ASSERT(!needsLayout());
     ASSERT(!simpleLineLayout());
@@ -1266,14 +1266,33 @@ LayoutRect RenderText::selectionRectForRepaint(const RenderLayerModelObject* rep
     if (startPos == endPos)
         return IntRect();
 
-    LayoutRect rect = m_lineBoxes.selectionRectForRange(startPos, endPos);
+    LayoutRect resultRect;
+    if (!rects)
+        resultRect = m_lineBoxes.selectionRectForRange(startPos, endPos);
+    else {
+        m_lineBoxes.collectSelectionRectsForRange(startPos, endPos, *rects);
+        for (auto& rect : *rects) {
+            resultRect.unite(rect);
+            rect = localToContainerQuad(FloatRect(rect), repaintContainer).enclosingBoundingBox();
+        }
+    }
 
     if (clipToVisibleContent)
-        computeRectForRepaint(repaintContainer, rect);
+        computeRectForRepaint(repaintContainer, resultRect);
     else
-        rect = localToContainerQuad(FloatRect(rect), repaintContainer).enclosingBoundingBox();
+        resultRect = localToContainerQuad(FloatRect(resultRect), repaintContainer).enclosingBoundingBox();
+
+    return resultRect;
+}
 
-    return rect;
+LayoutRect RenderText::collectSelectionRectsForLineBoxes(const RenderLayerModelObject* repaintContainer, bool clipToVisibleContent, Vector<LayoutRect>& rects)
+{
+    return collectSelectionRectsForLineBoxes(repaintContainer, clipToVisibleContent, &rects);
+}
+
+LayoutRect RenderText::selectionRectForRepaint(const RenderLayerModelObject* repaintContainer, bool clipToVisibleContent)
+{
+    return collectSelectionRectsForLineBoxes(repaintContainer, clipToVisibleContent, nullptr);
 }
 
 int RenderText::caretMinOffset() const
index b107989..f696630 100644 (file)
@@ -109,6 +109,8 @@ public:
     virtual LayoutRect selectionRectForRepaint(const RenderLayerModelObject* repaintContainer, bool clipToVisibleContent = true) override;
     virtual LayoutRect localCaretRect(InlineBox*, int caretOffset, LayoutUnit* extraWidthToEndOfLine = 0) override;
 
+    LayoutRect collectSelectionRectsForLineBoxes(const RenderLayerModelObject* repaintContainer, bool clipToVisibleContent, Vector<LayoutRect>& rects);
+
     LayoutUnit marginLeft() const { return minimumValueForLength(style().marginLeft(), 0); }
     LayoutUnit marginRight() const { return minimumValueForLength(style().marginRight(), 0); }
 
@@ -184,6 +186,8 @@ private:
 
     void secureText(UChar mask);
 
+    LayoutRect collectSelectionRectsForLineBoxes(const RenderLayerModelObject* repaintContainer, bool clipToVisibleContent, Vector<LayoutRect>*);
+
     void node() const = delete;
 
     // We put the bitfield first to minimize padding on 64-bit.
index 3ac68a4..b817da7 100644 (file)
@@ -495,6 +495,17 @@ LayoutRect RenderTextLineBoxes::selectionRectForRange(unsigned start, unsigned e
     return rect;
 }
 
+void RenderTextLineBoxes::collectSelectionRectsForRange(unsigned start, unsigned end, Vector<LayoutRect>& rects)
+{
+    for (auto box = m_first; box; box = box->nextTextBox()) {
+        LayoutRect rect;
+        rect.unite(box->localSelectionRect(start, end));
+        rect.unite(ellipsisRectForBox(*box, start, end));
+        if (!rect.size().isEmpty())
+            rects.append(rect);
+    }
+}
+
 Vector<IntRect> RenderTextLineBoxes::absoluteRects(const LayoutPoint& accumulatedOffset) const
 {
     Vector<IntRect> rects;
index d87c6b1..211e4a0 100644 (file)
@@ -68,6 +68,7 @@ public:
 
     void setSelectionState(RenderText&, RenderObject::SelectionState);
     LayoutRect selectionRectForRange(unsigned start, unsigned end);
+    void collectSelectionRectsForRange(unsigned start, unsigned end, Vector<LayoutRect>& rects);
 
     IntRect boundingBox(const RenderText&) const;
     IntPoint firstRunLocation() const;
index 26d52d0..f7e6e45 100644 (file)
@@ -992,7 +992,8 @@ void RenderView::setSubtreeSelection(SelectionSubtreeRoot& root, RenderObject* s
             std::unique_ptr<RenderSelectionInfo> selectionInfo = std::make_unique<RenderSelectionInfo>(o, true);
 
 #if ENABLE(SERVICE_CONTROLS)
-            m_selectionRectGatherer.addRect(selectionInfo->rect());
+            for (auto& rect : selectionInfo->rects())
+                m_selectionRectGatherer.addRect(rect);
 #endif
 
             newSelectedObjects.set(o, std::move(selectionInfo));
index 42c0179..502287a 100644 (file)
 
 2014-07-03  Brady Eidson  <beidson@apple.com>
 
+        Selection rects sent to ServicesOverlayController are wrong.
+        <rdar://problem/16727796> and https://bugs.webkit.org/show_bug.cgi?id=134568
+
+        Reviewed by Darin Adler (and Tim Horton and Ryosuke Niwa).
+
+        * WebProcess/WebCoreSupport/WebEditorClient.cpp:
+        (WebKit::WebEditorClient::selectionRectsDidChange): Also pass the GapRects to the ServicesOverlayController.
+        * WebProcess/WebCoreSupport/WebEditorClient.h:
+
+        * WebProcess/WebPage/ServicesOverlayController.h:
+        * WebProcess/WebPage/mac/ServicesOverlayController.mm:
+        (WebKit::expandForGap):
+        (WebKit::compactRectsWithGapRects): Combine 3+ rects down to exactly 3 rects, then expand them based on GapRects.
+        (WebKit::ServicesOverlayController::selectionRectsDidChange): Call compactRectsWithGapRects, then reverse the list.
+        (WebKit::ServicesOverlayController::drawSelectionHighlight): Tell data detectors to flip this.
+        (WebKit::ServicesOverlayController::drawTelephoneNumberHighlight): Tell data detectors to flip this.
+        (WebKit::ServicesOverlayController::drawCurrentHighlight): No need to flip this anymore.
+
+2014-07-03  Brady Eidson  <beidson@apple.com>
+
         Possible crash in IconDatabase in WebCore::IconDatabase::dispatchDidRemoveAllIconsOnMainThread
         <rdar://problem/17437687> and https://bugs.webkit.org/show_bug.cgi?id=134517
 
index d7d649d..bb14273 100644 (file)
@@ -531,11 +531,11 @@ void WebEditorClient::selectedTelephoneNumberRangesChanged(const Vector<RefPtr<R
     m_page->servicesOverlayController().selectedTelephoneNumberRangesChanged(ranges);
 #endif
 }
-void WebEditorClient::selectionRectsDidChange(const Vector<LayoutRect>& rects)
+void WebEditorClient::selectionRectsDidChange(const Vector<LayoutRect>& rects, const Vector<GapRects>& gapRects)
 {
 #if PLATFORM(MAC)
     if (m_page->serviceControlsEnabled())
-        m_page->servicesOverlayController().selectionRectsDidChange(rects);
+        m_page->servicesOverlayController().selectionRectsDidChange(rects, gapRects);
 #endif
 }
 #endif // ENABLE(SERVICE_CONTROLS) && ENABLE(TELEPHONE_NUMBER_DETECTION)
index 083b599..145d49b 100644 (file)
@@ -169,7 +169,7 @@ private:
 
 #if ENABLE(TELEPHONE_NUMBER_DETECTION) || ENABLE(SERVICE_CONTROLS)
     virtual void selectedTelephoneNumberRangesChanged(const Vector<RefPtr<WebCore::Range>>&) override;
-    virtual void selectionRectsDidChange(const Vector<WebCore::LayoutRect>&) override;
+    virtual void selectionRectsDidChange(const Vector<WebCore::LayoutRect>&, const Vector<WebCore::GapRects>&) override;
 #endif
 
     WebPage* m_page;
index 7b2dbf3..06f289a 100644 (file)
@@ -34,6 +34,8 @@ typedef void* DDHighlightRef;
 namespace WebCore {
 class LayoutRect;
 class Range;
+
+struct GapRects;
 }
 
 namespace WebKit {
@@ -48,7 +50,7 @@ public:
     ~ServicesOverlayController();
 
     void selectedTelephoneNumberRangesChanged(const Vector<RefPtr<WebCore::Range>>&);
-    void selectionRectsDidChange(const Vector<WebCore::LayoutRect>&);
+    void selectionRectsDidChange(const Vector<WebCore::LayoutRect>&, const Vector<WebCore::GapRects>&);
 
 private:
     void createOverlayIfNeeded();
index 6fb26d1..96d5f0a 100644 (file)
@@ -33,6 +33,7 @@
 #import <WebCore/Document.h>
 #import <WebCore/FloatQuad.h>
 #import <WebCore/FrameView.h>
+#import <WebCore/GapRects.h>
 #import <WebCore/GraphicsContext.h>
 #import <WebCore/MainFrame.h>
 #import <WebCore/SoftLinking.h>
@@ -49,10 +50,10 @@ typedef void* DDHighlightRef;
 
 typedef NSUInteger DDHighlightStyle;
 static const DDHighlightStyle DDHighlightNoOutlineWithArrow = (1 << 16);
+static const DDHighlightStyle DDHighlightOutlineWithArrow = (1 << 16) | 1;
 
 SOFT_LINK_PRIVATE_FRAMEWORK_OPTIONAL(DataDetectors)
-SOFT_LINK(DataDetectors, DDHighlightCreateWithRectsInVisibleRect, DDHighlightRef, (CFAllocatorRef allocator, CGRect * rects, CFIndex count, CGRect globalVisibleRect, Boolean withArrow), (allocator, rects, count, globalVisibleRect, withArrow))
-SOFT_LINK(DataDetectors, DDHighlightCreateWithRectsInVisibleRectWithStyle, DDHighlightRef, (CFAllocatorRef allocator, CGRect * rects, CFIndex count, CGRect globalVisibleRect, DDHighlightStyle style, Boolean withArrow), (allocator, rects, count, globalVisibleRect, style, withArrow))
+SOFT_LINK(DataDetectors, DDHighlightCreateWithRectsInVisibleRectWithStyleAndDirection, DDHighlightRef, (CFAllocatorRef allocator, CGRect* rects, CFIndex count, CGRect globalVisibleRect, DDHighlightStyle style, Boolean withArrow, NSWritingDirection writingDirection, Boolean endsWithEOL, Boolean flipped), (allocator, rects, count, globalVisibleRect, style, withArrow, writingDirection, endsWithEOL, flipped))
 SOFT_LINK(DataDetectors, DDHighlightGetLayerWithContext, CGLayerRef, (DDHighlightRef highlight, CGContextRef context), (highlight, context))
 SOFT_LINK(DataDetectors, DDHighlightGetBoundingRect, CGRect, (DDHighlightRef highlight), (highlight))
 SOFT_LINK(DataDetectors, DDHighlightPointIsOnHighlight, Boolean, (DDHighlightRef highlight, CGPoint point, Boolean* onButton), (highlight, point, onButton))
@@ -128,12 +129,105 @@ void ServicesOverlayController::createOverlayIfNeeded()
     m_servicesOverlay->setNeedsDisplay();
 }
 
-void ServicesOverlayController::selectionRectsDidChange(const Vector<LayoutRect>& rects)
+static const uint8_t AlignmentNone = 0;
+static const uint8_t AlignmentLeft = 1 << 0;
+static const uint8_t AlignmentRight = 1 << 1;
+
+static void expandForGap(Vector<LayoutRect>& rects, uint8_t* alignments, const GapRects& gap)
+{
+    if (!gap.left().isEmpty()) {
+        LayoutUnit leftEdge = gap.left().x();
+        for (unsigned i = 0; i < 3; ++i) {
+            if (alignments[i] & AlignmentLeft)
+                rects[i].shiftXEdgeTo(leftEdge);
+        }
+    }
+
+    if (!gap.right().isEmpty()) {
+        LayoutUnit rightEdge = gap.right().maxX();
+        for (unsigned i = 0; i < 3; ++i) {
+            if (alignments[i] & AlignmentRight)
+                rects[i].shiftMaxXEdgeTo(rightEdge);
+        }
+    }
+}
+
+static void compactRectsWithGapRects(Vector<LayoutRect>& rects, const Vector<GapRects>& gapRects)
+{
+    if (rects.isEmpty())
+        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);
+    }
+
+    // FIXME: The following alignments are correct for LTR text.
+    // We should also account for RTL.
+    uint8_t alignments[3];
+    if (rects.size() == 1) {
+        alignments[0] = AlignmentLeft | AlignmentRight;
+        alignments[1] = AlignmentNone;
+        alignments[2] = AlignmentNone;
+    } else if (rects.size() == 2) {
+        alignments[0] = AlignmentRight;
+        alignments[1] = AlignmentLeft;
+        alignments[2] = AlignmentNone;
+    } else {
+        alignments[0] = AlignmentRight;
+        alignments[1] = AlignmentLeft | AlignmentRight;
+        alignments[2] = AlignmentLeft;
+    }
+
+    // Account for each GapRects by extending the edge of certain LayoutRects to meet the gap.
+    for (auto& gap : gapRects)
+        expandForGap(rects, alignments, gap);
+
+    // If we have 3 rects we might need one final GapRects to align the edges.
+    if (rects.size() == 3) {
+        LayoutRect left;
+        LayoutRect right;
+        for (unsigned i = 0; i < 3; ++i) {
+            if (alignments[i] & AlignmentLeft) {
+                if (left.isEmpty())
+                    left = rects[i];
+                else if (rects[i].x() < left.x())
+                    left = rects[i];
+            }
+            if (alignments[i] & AlignmentRight) {
+                if (right.isEmpty())
+                    right = rects[i];
+                else if ((rects[i].x() + rects[i].width()) > (right.x() + right.width()))
+                    right = rects[i];
+            }
+        }
+
+        if (!left.isEmpty() || !right.isEmpty()) {
+            GapRects gap;
+            gap.uniteLeft(left);
+            gap.uniteRight(right);
+            expandForGap(rects, alignments, gap);
+        }
+    }
+}
+
+void ServicesOverlayController::selectionRectsDidChange(const Vector<LayoutRect>& rects, const Vector<GapRects>& gapRects)
 {
 #if __MAC_OS_X_VERSION_MIN_REQUIRED > 1090
     m_currentHighlightIsDirty = true;
     m_currentSelectionRects = rects;
 
+    compactRectsWithGapRects(m_currentSelectionRects, gapRects);
+
+    // DataDetectors needs these reversed in order to place the arrow in the right location.
+    m_currentSelectionRects.reverse();
+
     createOverlayIfNeeded();
 #else
     UNUSED_PARAM(rects);
@@ -205,12 +299,13 @@ void ServicesOverlayController::drawSelectionHighlight(WebCore::GraphicsContext&
 
         if (!cgRects.isEmpty()) {
             CGRect bounds = m_webPage->corePage()->mainFrame().view()->boundsRect();
-            m_currentHighlight = adoptCF(DDHighlightCreateWithRectsInVisibleRectWithStyle(nullptr, cgRects.begin(), cgRects.size(), bounds, DDHighlightNoOutlineWithArrow, true));
+            m_currentHighlight = adoptCF(DDHighlightCreateWithRectsInVisibleRectWithStyleAndDirection(nullptr, cgRects.begin(), cgRects.size(), bounds, DDHighlightNoOutlineWithArrow, YES, NSWritingDirectionNatural, NO, YES));
             m_currentHighlightIsDirty = false;
         }
     }
 
-    drawCurrentHighlight(graphicsContext);
+    if (m_currentHighlight)
+        drawCurrentHighlight(graphicsContext);
 }
 
 void ServicesOverlayController::drawTelephoneNumberHighlight(WebCore::GraphicsContext& graphicsContext, const WebCore::IntRect& dirtyRect)
@@ -241,15 +336,18 @@ void ServicesOverlayController::drawTelephoneNumberHighlight(WebCore::GraphicsCo
     if (!m_currentHighlight || m_currentHighlightIsDirty) {
         CGRect cgRect = (CGRect)rect;
 
-        m_currentHighlight = adoptCF(DDHighlightCreateWithRectsInVisibleRect(nullptr, &cgRect, 1, viewForRange->boundsRect(), true));
+        m_currentHighlight = adoptCF(DDHighlightCreateWithRectsInVisibleRectWithStyleAndDirection(nullptr, &cgRect, 1, viewForRange->boundsRect(), DDHighlightOutlineWithArrow, YES, NSWritingDirectionNatural, NO, YES));
         m_currentHighlightIsDirty = false;
     }
 
-    drawCurrentHighlight(graphicsContext);
+    if (m_currentHighlight)
+        drawCurrentHighlight(graphicsContext);
 }
 
 void ServicesOverlayController::drawCurrentHighlight(WebCore::GraphicsContext& graphicsContext)
 {
+    ASSERT(m_currentHighlight);
+
     Boolean onButton;
     m_mouseIsOverHighlight = DDHighlightPointIsOnHighlight(m_currentHighlight.get(), (CGPoint)m_mousePosition, &onButton);
 
@@ -265,9 +363,7 @@ void ServicesOverlayController::drawCurrentHighlight(WebCore::GraphicsContext& g
     GraphicsContextStateSaver stateSaver(graphicsContext);
 
     graphicsContext.translate(toFloatSize(highlightBoundingRect.origin));
-    graphicsContext.scale(FloatSize(1, -1));
-    graphicsContext.translate(FloatSize(0, -highlightBoundingRect.size.height));
-    
+
     CGRect highlightDrawRect = highlightBoundingRect;
     highlightDrawRect.origin.x = 0;
     highlightDrawRect.origin.y = 0;