WebFrame::selectRange and moveCaret should behave like mouse selection

Source/WebKit/chromium/src/WebFrameImpl.cpp

Index: Source/WebKit/chromium/src/WebFrameImpl.cpp
diff --git a/Source/WebKit/chromium/src/WebFrameImpl.cpp b/Source/WebKit/chromium/src/WebFrameImpl.cpp
index 7546f49b3342613df44397dcbe7c6f04d9eef867..7e9d81fa50cfa00ae616223d995a9b5756b1c3e9 100644
--- a/Source/WebKit/chromium/src/WebFrameImpl.cpp
+++ b/Source/WebKit/chromium/src/WebFrameImpl.cpp
@@ -1363,15 +1363,7 @@ bool WebFrameImpl::selectWordAroundCaret()
 
 void WebFrameImpl::selectRange(const WebPoint& base, const WebPoint& extent)
 {
-    IntPoint unscaledBase = base;
-    IntPoint unscaledExtent = extent;
-    unscaledExtent.scale(1 / view()->pageScaleFactor(), 1 / view()->pageScaleFactor());
-    unscaledBase.scale(1 / view()->pageScaleFactor(), 1 / view()->pageScaleFactor());
-    VisiblePosition basePosition = visiblePositionForWindowPoint(unscaledBase);
-    VisiblePosition extentPosition = visiblePositionForWindowPoint(unscaledExtent);
-    VisibleSelection newSelection = VisibleSelection(basePosition, extentPosition);
-    if (frame()->selection()->shouldChangeSelection(newSelection))
-        frame()->selection()->setSelection(newSelection, CharacterGranularity);
+    moveRangeSelection(base, extent);
 }
 
 void WebFrameImpl::selectRange(const WebRange& webRange)
@@ -1382,31 +1374,45 @@ void WebFrameImpl::selectRange(const WebRange& webRange)
 
 void WebFrameImpl::moveCaretSelectionTowardsWindowPoint(const WebPoint& point)
 {
-    IntPoint unscaledPoint(point);
-    unscaledPoint.scale(1 / view()->pageScaleFactor(), 1 / view()->pageScaleFactor());
+    moveCaretSelection(point);
+}
+
+void WebFrameImpl::moveRangeSelection(const WebPoint& base, const WebPoint& extent) {
+    FrameSelection* selection = frame()->selection();
+    if (!selection)
+        return;
+
+    RenderObject* renderer;
+    if (Element* editable = selection->rootEditableElement())
+        renderer = editable->renderer();
+    else
+        renderer = frame()->document()->renderer();
 
+    VisiblePosition basePosition = visiblePositionForWindowPoint(base, renderer);
+    VisiblePosition extentPosition = visiblePositionForWindowPoint(extent, renderer);
+    VisibleSelection newSelection = VisibleSelection(basePosition, extentPosition);
+    if (frame()->selection()->shouldChangeSelection(newSelection))
+        frame()->selection()->setSelection(newSelection, CharacterGranularity);
+}
+
+void WebFrameImpl::moveCaretSelection(const WebPoint& point) {
     Element* editable = frame()->selection()->rootEditableElement();
     if (!editable)
         return;
 
-    IntPoint contentsPoint = frame()->view()->windowToContents(unscaledPoint);
-    LayoutPoint localPoint(editable->convertFromPage(contentsPoint));
-    VisiblePosition position = editable->renderer()->positionForPoint(localPoint);
+    VisiblePosition position = visiblePositionForWindowPoint(point, editable->renderer());
+
     if (frame()->selection()->shouldChangeSelection(position))
         frame()->selection()->moveTo(position, UserTriggered);
 }
 
-VisiblePosition WebFrameImpl::visiblePositionForWindowPoint(const WebPoint& point)
+VisiblePosition WebFrameImpl::visiblePositionForWindowPoint(const WebPoint& point, RenderObject* renderer)

[leviw_travelin_and_unemployed, 2013/05/06 22:38:47]

I'd give this a more descriptive name than renderer... something like
rootRenderer perhaps?

 {
-    HitTestRequest request = HitTestRequest::Move | HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::IgnoreClipping | HitTestRequest::DisallowShadowContent;
-    HitTestResult result(frame()->view()->windowToContents(IntPoint(point)));
-
-    frame()->document()->renderView()->layer()->hitTest(request, result);
-
-    Node* node = result.targetNode();
-    if (!node)
-        return VisiblePosition();
-    return node->renderer()->positionForPoint(result.localPoint());
+    FloatPoint unscaledPoint(point);
+    unscaledPoint.scale(1 / view()->pageScaleFactor(), 1 / view()->pageScaleFactor());
+    IntPoint contentsPoint = frame()->view()->windowToContents(roundedIntPoint(unscaledPoint));
+    LayoutPoint localPoint(renderer->absoluteToLocal(contentsPoint, UseTransforms));
+    return renderer->positionForPoint(localPoint);

[leviw_travelin_and_unemployed, 2013/05/06 22:38:47]

Your issue is that positionForPoint isn't really meant to be used this way for
inlines. That hit-testing is actually handled by
RenderBlock::positionForPointWithInlineChildren. I believe this code would be
better off letting positionForPoint from the root do the work, then checking to
see if the result is a descendant of the root editable element you want instead.

 }
 
 int WebFrameImpl::printBegin(const WebPrintParams& printParams, const WebNode& constrainToNode, bool* useBrowserOverlays)