Selection services menu dropdown is in the wrong place when selecting some text on...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Aug 2014 00:28:25 +0000 (00:28 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Aug 2014 00:28:25 +0000 (00:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135582
<rdar://problem/17837636>

Reviewed by Simon Fraser.

* editing/SelectionRectGatherer.cpp:
(WebCore::SelectionRectGatherer::addRect):
(WebCore::SelectionRectGatherer::addGapRects):
(WebCore::SelectionRectGatherer::addRects): Deleted.
Rename addRects to addGapRects for clarity.
Map rects and gapRects to absolute RenderView coordinates so that
they are in a form WebKit2 can use. Previously they were sometimes
relative to a different repaint container, but that information was
lost when moving through SelectionRectGatherer.

Ideally we would keep selection rects as full quads instead of rects
for more of their life, but that problem is much deeper than just SelectionRectGatherer.

* editing/SelectionRectGatherer.h:
Add a comment clarifying the coordinate space of the stored selection rects.

* rendering/RenderView.cpp:
(WebCore::RenderView::applySubtreeSelection):
Rename addRects to addGapRects for clarity.

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

Source/WebCore/ChangeLog
Source/WebCore/editing/SelectionRectGatherer.cpp
Source/WebCore/editing/SelectionRectGatherer.h
Source/WebCore/rendering/RenderView.cpp

index 9c8c748ee5dab5279d9c18c8b5e6644a53154143..d190cfde1c8974effb6fc5ee360cc2a9bf7ab025 100644 (file)
@@ -1,3 +1,31 @@
+2014-08-04  Tim Horton  <timothy_horton@apple.com>
+
+        Selection services menu dropdown is in the wrong place when selecting some text on Yelp
+        https://bugs.webkit.org/show_bug.cgi?id=135582
+        <rdar://problem/17837636>
+
+        Reviewed by Simon Fraser.
+
+        * editing/SelectionRectGatherer.cpp:
+        (WebCore::SelectionRectGatherer::addRect):
+        (WebCore::SelectionRectGatherer::addGapRects):
+        (WebCore::SelectionRectGatherer::addRects): Deleted.
+        Rename addRects to addGapRects for clarity.
+        Map rects and gapRects to absolute RenderView coordinates so that
+        they are in a form WebKit2 can use. Previously they were sometimes
+        relative to a different repaint container, but that information was
+        lost when moving through SelectionRectGatherer.
+
+        Ideally we would keep selection rects as full quads instead of rects
+        for more of their life, but that problem is much deeper than just SelectionRectGatherer.
+
+        * editing/SelectionRectGatherer.h:
+        Add a comment clarifying the coordinate space of the stored selection rects.
+
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::applySubtreeSelection):
+        Rename addRects to addGapRects for clarity.
+
 2014-08-04  Bem Jones-Bey  <bjonesbe@adobe.com>
 
         [CSS Shapes] shape-margin not respected when it extends beyond an explicitly set margin
index 5a7ddb894fdb4c5623e9e8c4391aed6bde65fe75..1e77809e65cfca12c21e7aafa391ec811f87c882 100644 (file)
@@ -40,15 +40,21 @@ SelectionRectGatherer::SelectionRectGatherer(RenderView& renderView)
 {
 }
 
-void SelectionRectGatherer::addRect(const LayoutRect& rect)
+void SelectionRectGatherer::addRect(RenderLayerModelObject *repaintContainer, const LayoutRect& rect)
 {
-    if (!rect.isEmpty())
-        m_rects.append(rect);
+    if (!rect.isEmpty()) {
+        LayoutRect absoluteRect(repaintContainer->localToAbsoluteQuad(FloatQuad(rect)).boundingBox());
+        m_rects.append(absoluteRect);
+    }
 }
 
-void SelectionRectGatherer::addRects(const GapRects& rects)
+void SelectionRectGatherer::addGapRects(RenderLayerModelObject *repaintContainer, const GapRects& rects)
 {
-    m_gapRects.append(rects);
+    GapRects absoluteGapRects;
+    absoluteGapRects.uniteLeft(LayoutRect(repaintContainer->localToAbsoluteQuad(FloatQuad(rects.left())).boundingBox()));
+    absoluteGapRects.uniteCenter(LayoutRect(repaintContainer->localToAbsoluteQuad(FloatQuad(rects.center())).boundingBox()));
+    absoluteGapRects.uniteRight(LayoutRect(repaintContainer->localToAbsoluteQuad(FloatQuad(rects.right())).boundingBox()));
+    m_gapRects.append(absoluteGapRects);
 }
 
 SelectionRectGatherer::Notifier::Notifier(SelectionRectGatherer& gatherer)
index f87e52f73a8b868c4463d22161251ebd9ceda949..d36e47f4bb95828d163fd54d679b6bbd5e6388d7 100644 (file)
@@ -34,6 +34,7 @@
 namespace WebCore {
 
 class LayoutRect;
+class RenderLayerModelObject;
 class RenderView;
 
 struct GapRects;
@@ -44,8 +45,8 @@ class SelectionRectGatherer {
 public:
     SelectionRectGatherer(RenderView&);
 
-    void addRect(const LayoutRect&);
-    void addRects(const GapRects&);
+    void addRect(RenderLayerModelObject *repaintContainer, const LayoutRect&);
+    void addGapRects(RenderLayerModelObject *repaintContainer, const GapRects&);
 
     class Notifier {
         WTF_MAKE_NONCOPYABLE(Notifier);
@@ -61,6 +62,8 @@ public:
     
 private:
     RenderView& m_renderView;
+
+    // All rects are in RenderView coordinates.
     Vector<LayoutRect> m_rects;
     Vector<GapRects> m_gapRects;
 };
index 2f26605eaafbf867f89897bb962e38ef5713636b..3131578c8a036518f1459fdc0deeb823aba7de11 100644 (file)
@@ -1013,7 +1013,7 @@ void RenderView::applySubtreeSelection(SelectionSubtreeRoot& root, RenderObject*
 
 #if ENABLE(SERVICE_CONTROLS)
             for (auto& rect : selectionInfo->collectedSelectionRects())
-                m_selectionRectGatherer.addRect(rect);
+                m_selectionRectGatherer.addRect(selectionInfo->repaintContainer(), rect);
 #endif
 
             newSelectedObjects.set(o, WTF::move(selectionInfo));
@@ -1027,7 +1027,7 @@ void RenderView::applySubtreeSelection(SelectionSubtreeRoot& root, RenderObject*
                 cb = cb->containingBlock();
 
 #if ENABLE(SERVICE_CONTROLS)
-                m_selectionRectGatherer.addRects(blockInfo->rects());
+                m_selectionRectGatherer.addGapRects(blockInfo->repaintContainer(), blockInfo->rects());
 #endif
             }
         }