WebFrame::selectRange and moveCaret should behave like mouse selection

Source/WebKit/chromium/src/WebFrameImpl.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 1345 matching lines @@
     FrameSelection* selection = frame()->selection();
     ASSERT(!selection->isNone());
     if (selection->isNone() || selection->isRange())
         return false;
     selectWordAroundPosition(frame(), selection->selection().visibleStart());
     return true;
 }
 
 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)
 {
     if (RefPtr<Range> range = static_cast<PassRefPtr<Range> >(webRange))
         frame()->selection()->setSelectedRange(range.get(), WebCore::VP_DEFAULT_AFFINITY, false);
 }
 
 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)
 {
     ASSERT(!frame()->document()->isFrameSet());
     WebPluginContainerImpl* pluginContainer = 0;
     if (constrainToNode.isNull()) {
         // If this is a plugin document, check if the plugin supports its own
         // printing. If it does, we will delegate all printing to that.
         pluginContainer = pluginContainerFromFrame(frame());
@@ skipping 1079 matching lines @@
 
         // There is a possibility that the frame being detached was the only
         // pending one. We need to make sure final replies can be sent.
         flushCurrentScopingEffort(m_findRequestIdentifier);
 
         cancelPendingScopingEffort();
     }
 }
 
 } // namespace WebKit