From 50ae2ff1ef5765dd1735bee360f0880373bd9735 Mon Sep 17 00:00:00 2001 From: "cjhopman@chromium.org" Date: Wed, 16 Jan 2013 18:55:46 +0000 Subject: Merge 176454 > Add a new code/message path for moving an insertion handle > > On Android, we've been using selectRange for moving an insertion handle. > However, we don't really want the behavior of selectRange (for example, > dragging an insertion handle outside of a text box should not actual do > a "selectRange" on the position outside of the text box). The behavior > that we want has been added to WebKit as > WebFrame::moveCaretSelectionTowardsWindowPoint. > > This change introduces the code/message path from the Browser side > InsertionHandleController to WebKit similar to the current SelectRange > path. > > BUG=165244,163979,165661 > > > Review URL: https://chromiumcodereview.appspot.com/11593011 TBR=cjhopman@chromium.org Review URL: https://codereview.chromium.org/11962020 git-svn-id: svn://svn.chromium.org/chrome/branches/1364/src@177185 0039d316-1c4b-4281-b951-d872f2087c98 --- content/browser/android/content_view_core_impl.cc | 7 + content/browser/android/content_view_core_impl.h | 1 + .../renderer_host/render_widget_host_impl.cc | 24 +++ content/common/view_messages.h | 6 + .../chromium/content/browser/ContentViewCore.java | 4 +- .../content/browser/InsertionHandleTest.java | 163 ++++++++++++++++++--- .../content/browser/test/util/TouchUtils.java | 2 +- content/renderer/render_view_impl.cc | 10 ++ content/renderer/render_view_impl.h | 1 + .../device_files/insertion_handle/input_text.html | 7 + 10 files changed, 206 insertions(+), 19 deletions(-) create mode 100644 content/test/data/android/device_files/insertion_handle/input_text.html diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc index 9509039..73e905a 100644 --- a/content/browser/android/content_view_core_impl.cc +++ b/content/browser/android/content_view_core_impl.cc @@ -1014,6 +1014,13 @@ void ContentViewCoreImpl::SelectBetweenCoordinates(JNIEnv* env, jobject obj, } } +void ContentViewCoreImpl::MoveCaret(JNIEnv* env, jobject obj, + jint x, jint y) { + if (GetRenderWidgetHostViewAndroid()) { + GetRenderWidgetHostViewAndroid()->MoveCaret(gfx::Point(x, y)); + } +} + jboolean ContentViewCoreImpl::CanGoBack(JNIEnv* env, jobject obj) { return web_contents_->GetController().CanGoBack(); } diff --git a/content/browser/android/content_view_core_impl.h b/content/browser/android/content_view_core_impl.h index c45462c..cd70fd1 100644 --- a/content/browser/android/content_view_core_impl.h +++ b/content/browser/android/content_view_core_impl.h @@ -144,6 +144,7 @@ class ContentViewCoreImpl : public ContentViewCore, void SelectBetweenCoordinates(JNIEnv* env, jobject obj, jint x1, jint y1, jint x2, jint y2); + void MoveCaret(JNIEnv* env, jobject obj, jint x, jint y); jboolean CanGoBack(JNIEnv* env, jobject obj); jboolean CanGoForward(JNIEnv* env, jobject obj); diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc index de4ab7f..70f0e97 100644 --- a/content/browser/renderer_host/render_widget_host_impl.cc +++ b/content/browser/renderer_host/render_widget_host_impl.cc @@ -138,6 +138,7 @@ RenderWidgetHostImpl::RenderWidgetHostImpl(RenderWidgetHostDelegate* delegate, mouse_move_pending_(false), mouse_wheel_pending_(false), select_range_pending_(false), + move_caret_pending_(false), needs_repainting_on_restore_(false), is_unresponsive_(false), in_flight_event_count_(0), @@ -340,6 +341,7 @@ bool RenderWidgetHostImpl::OnMessageReceived(const IPC::Message &msg) { IPC_MESSAGE_HANDLER(ViewHostMsg_HandleInputEvent_ACK, OnMsgInputEventAck) IPC_MESSAGE_HANDLER(ViewHostMsg_BeginSmoothScroll, OnMsgBeginSmoothScroll) IPC_MESSAGE_HANDLER(ViewHostMsg_SelectRange_ACK, OnMsgSelectRangeAck) + IPC_MESSAGE_HANDLER(ViewHostMsg_MoveCaret_ACK, OnMsgMoveCaretAck) IPC_MESSAGE_HANDLER(ViewHostMsg_Focus, OnMsgFocus) IPC_MESSAGE_HANDLER(ViewHostMsg_Blur, OnMsgBlur) IPC_MESSAGE_HANDLER(ViewHostMsg_HasTouchEventHandlers, @@ -1219,6 +1221,10 @@ void RenderWidgetHostImpl::RendererExited(base::TerminationStatus status, select_range_pending_ = false; next_selection_range_.reset(); + // Must reset these to ensure that MoveCaret works with a new renderer. + move_caret_pending_ = false; + next_move_caret_.reset(); + touch_event_queue_->Reset(); // Must reset these to ensure that gesture events work with a new renderer. @@ -1855,6 +1861,14 @@ void RenderWidgetHostImpl::OnMsgSelectRangeAck() { } } +void RenderWidgetHostImpl::OnMsgMoveCaretAck() { + move_caret_pending_ = false; + if (next_move_caret_.get()) { + scoped_ptr next(next_move_caret_.Pass()); + MoveCaret(*next); + } +} + void RenderWidgetHostImpl::ProcessWheelAck(bool processed) { mouse_wheel_pending_ = false; @@ -2224,6 +2238,16 @@ void RenderWidgetHostImpl::SelectRange(const gfx::Point& start, Send(new ViewMsg_SelectRange(GetRoutingID(), start, end)); } +void RenderWidgetHostImpl::MoveCaret(const gfx::Point& point) { + if (move_caret_pending_) { + next_move_caret_.reset(new gfx::Point(point)); + return; + } + + move_caret_pending_ = true; + Send(new ViewMsg_MoveCaret(GetRoutingID(), point)); +} + void RenderWidgetHostImpl::Undo() { Send(new ViewMsg_Undo(GetRoutingID())); RecordAction(UserMetricsAction("Undo")); diff --git a/content/common/view_messages.h b/content/common/view_messages.h index 2ea456d..64a9441 100644 --- a/content/common/view_messages.h +++ b/content/common/view_messages.h @@ -1033,6 +1033,11 @@ IPC_MESSAGE_ROUTED2(ViewMsg_SelectRange, gfx::Point /* start */, gfx::Point /* end */) +// Requests the renderer to move the caret selection toward the point. +// Expects a MoveCaret_ACK message when finished. +IPC_MESSAGE_ROUTED1(ViewMsg_MoveCaret, + gfx::Point /* location */) + // Copies the image at location x, y to the clipboard (if there indeed is an // image at that location). IPC_MESSAGE_ROUTED2(ViewMsg_CopyImageAt, @@ -2044,6 +2049,7 @@ IPC_MESSAGE_ROUTED2(ViewHostMsg_SetTooltipText, WebKit::WebTextDirection /* text direction hint */) IPC_MESSAGE_ROUTED0(ViewHostMsg_SelectRange_ACK) +IPC_MESSAGE_ROUTED0(ViewHostMsg_MoveCaret_ACK) // Notification that the text selection has changed. // Note: The secound parameter is the character based offset of the string16 diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java index ede5628..b38ce55 100644 --- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java +++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java @@ -1689,7 +1689,7 @@ public class ContentViewCore implements MotionEventDelegate { @Override public void setCursorPosition(int x, int y) { if (mNativeContentViewCore != 0) { - nativeSelectBetweenCoordinates(mNativeContentViewCore, x, y, x, y); + nativeMoveCaret(mNativeContentViewCore, x, y); } } @@ -2484,6 +2484,8 @@ public class ContentViewCore implements MotionEventDelegate { private native void nativeSelectBetweenCoordinates( int nativeContentViewCoreImpl, int x1, int y1, int x2, int y2); + private native void nativeMoveCaret(int nativeContentViewCoreImpl, int x, int y); + private native boolean nativeCanGoBack(int nativeContentViewCoreImpl); private native boolean nativeCanGoForward(int nativeContentViewCoreImpl); diff --git a/content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java b/content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java index c1da521..8d207a3 100644 --- a/content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java +++ b/content/public/android/javatests/src/org/chromium/content/browser/InsertionHandleTest.java @@ -14,12 +14,16 @@ import android.text.Selection; import org.chromium.base.test.util.Feature; import org.chromium.content.browser.test.util.Criteria; import org.chromium.content.browser.test.util.CriteriaHelper; +import org.chromium.content.browser.test.util.DOMUtils; +import org.chromium.content.browser.test.util.TestCallbackHelperContainer; import org.chromium.content.browser.test.util.TouchUtils; import org.chromium.content_shell.ContentShellTestBase; public class InsertionHandleTest extends ContentShellTestBase { private static final String FILENAME = "content/insertion_handle/editable_long_text.html"; + private static final String INPUT_TEXT_FILENAME = "content/insertion_handle/input_text.html"; + private static final String INPUT_TEXT_ID = "input_text"; // Offset to compensate for the fact that the handle is below the text. private static final int VERTICAL_OFFSET = 10; // These positions should both be in the text area and not within @@ -28,9 +32,43 @@ public class InsertionHandleTest extends ContentShellTestBase { private static final int INITIAL_CLICK_Y = 100; private static final int DRAG_TO_X = 287; private static final int DRAG_TO_Y = 199; - private static final int HANDLE_POSITION_TOLERANCE = 60; + private static final int HANDLE_POSITION_TOLERANCE = 40; private static final String PASTE_TEXT = "**test text to paste**"; + private void dragHandleTo(int dragToX, int dragToY, int steps) throws Throwable { + InsertionHandleController ihc = getContentViewCore().getInsertionHandleControllerForTest(); + HandleView handle = ihc.getHandleViewForTest(); + int initialX = handle.getPositionX(); + int initialY = handle.getPositionY() + VERTICAL_OFFSET; + ContentView view = getContentView(); + + int fromLocation[] = TouchUtils.getAbsoluteLocationFromRelative(view, initialX, initialY); + int toLocation[] = TouchUtils.getAbsoluteLocationFromRelative(view, dragToX, dragToY); + + long downTime = TouchUtils.dragStart(getInstrumentation(), fromLocation[0], + fromLocation[1]); + assertWaitForHandleDraggingEquals(true); + TouchUtils.dragTo(getInstrumentation(), fromLocation[0], toLocation[0], fromLocation[1], + toLocation[1], steps, downTime); + TouchUtils.dragEnd(getInstrumentation(), toLocation[0], toLocation[1], downTime); + assertWaitForHandleDraggingEquals(false); + } + + private void dragHandleTo(int dragToX, int dragToY) throws Throwable { + dragHandleTo(dragToX, dragToY, 5); + } + + private void assertWaitForHandleDraggingEquals(final boolean expected) throws Throwable { + InsertionHandleController ihc = getContentViewCore().getInsertionHandleControllerForTest(); + final HandleView handle = ihc.getHandleViewForTest(); + assertTrue(CriteriaHelper.pollForCriteria(new Criteria() { + @Override + public boolean isSatisfied() { + return handle.isDragging() == expected; + } + })); + } + @MediumTest @Feature({"TextSelection", "TextInput", "Main"}) public void testDragInsertionHandle() throws Throwable { @@ -38,15 +76,8 @@ public class InsertionHandleTest extends ContentShellTestBase { clickToShowInsertionHandle(); - InsertionHandleController ihc = getContentViewCore().getInsertionHandleControllerForTest(); - HandleView handle = ihc.getHandleViewForTest(); - int initialX = handle.getPositionX(); - int initialY = handle.getPositionY(); - - TouchUtils.dragCompleteView(getInstrumentation(), getContentView(), initialX, DRAG_TO_X, - initialY + VERTICAL_OFFSET, DRAG_TO_Y, 5); - - assertTrue(waitForHandleNear(handle, DRAG_TO_X, DRAG_TO_Y)); + dragHandleTo(DRAG_TO_X, DRAG_TO_Y); + assertWaitForHandleNear(DRAG_TO_X, DRAG_TO_Y); // Unselecting should cause the handle to disappear. getImeAdapter().unselect(); @@ -58,14 +89,15 @@ public class InsertionHandleTest extends ContentShellTestBase { (Math.abs(handle.getPositionY() - y) < HANDLE_POSITION_TOLERANCE); } - private static boolean waitForHandleNear(final HandleView handle, final int x, - final int y) throws Throwable { - return CriteriaHelper.pollForCriteria(new Criteria() { + private void assertWaitForHandleNear(final int x, final int y) throws Throwable { + InsertionHandleController ihc = getContentViewCore().getInsertionHandleControllerForTest(); + final HandleView handle = ihc.getHandleViewForTest(); + assertTrue(CriteriaHelper.pollForCriteria(new Criteria() { @Override public boolean isSatisfied() { return isHandleNear(handle, x, y); } - }); + })); } private boolean waitForHandleShowingEquals(final boolean shouldBeShowing) @@ -138,6 +170,88 @@ public class InsertionHandleTest extends ContentShellTestBase { }); } + @MediumTest + @Feature({"TextSelection", "TextInput", "Main"}) + public void testDragInsertionHandleInputText() throws Throwable { + startActivityWithTestUrl(INPUT_TEXT_FILENAME); + + DOMUtils.clickNode(this, getContentView(), + new TestCallbackHelperContainer(getContentView()), INPUT_TEXT_ID); + TouchUtils.sleepForDoubleTapTimeout(getInstrumentation()); + DOMUtils.clickNode(this, getContentView(), + new TestCallbackHelperContainer(getContentView()), INPUT_TEXT_ID); + + assertTrue(waitForHandleShowingEquals(true)); + assertWaitForZoomFinished(); + assertTrue(waitForHandleViewStopped()); + + InsertionHandleController ihc = getContentViewCore().getInsertionHandleControllerForTest(); + HandleView handle = ihc.getHandleViewForTest(); + + int initialX = handle.getPositionX(); + int initialY = handle.getPositionY() + VERTICAL_OFFSET; + int dragToX = initialX + 120; + int dragToY = initialY; + dragHandleTo(dragToX, dragToY); + + assertTrue(waitForHandleViewStopped()); + assertWaitForHandleNear(dragToX, initialY); + + TouchUtils.sleepForDoubleTapTimeout(getInstrumentation()); + + initialX = handle.getPositionX(); + initialY = handle.getPositionY() + VERTICAL_OFFSET; + dragToX = initialX - 120; + dragToY = initialY; + dragHandleTo(dragToX, dragToY); + + assertTrue(waitForHandleViewStopped()); + // Vertical drag should not change the y-position. + assertWaitForHandleNear(dragToX, initialY); + } + + @MediumTest + @Feature({"TextSelection", "TextInput", "Main"}) + public void testDragInsertionHandleInputTextOutsideBounds() throws Throwable { + startActivityWithTestUrl(INPUT_TEXT_FILENAME); + + DOMUtils.clickNode(this, getContentView(), + new TestCallbackHelperContainer(getContentView()), INPUT_TEXT_ID); + TouchUtils.sleepForDoubleTapTimeout(getInstrumentation()); + DOMUtils.clickNode(this, getContentView(), + new TestCallbackHelperContainer(getContentView()), INPUT_TEXT_ID); + + assertTrue(waitForHandleShowingEquals(true)); + assertWaitForZoomFinished(); + + InsertionHandleController ihc = getContentViewCore().getInsertionHandleControllerForTest(); + HandleView handle = ihc.getHandleViewForTest(); + + getContentView().zoomReset(); + assertWaitForZoomFinished(); + + // Quickly (i.e. few move events) drag the handle down and to the right. When this happens, + // the handle should stay vertically aligned with the drag position. + int initialX = handle.getPositionX(); + int initialY = handle.getPositionY() + VERTICAL_OFFSET; + int dragToX = initialX; + int dragToY = initialY + 150; + + // A vertical drag should not move the insertion handle. + dragHandleTo(dragToX, dragToY); + assertTrue(waitForHandleViewStopped()); + // TODO(cjhopman): This currently does not work, dragging above or below will snap to the + // beginning/end of the editable. See http://crbug.com/169055 + //assertWaitForHandleNear(initialX, initialY); + + // The input box does not go to the edge of the screen, and neither should the insertion + // handle. + dragToX = getContentView().getWidth(); + dragHandleTo(dragToX, dragToY); + assertTrue(waitForHandleViewStopped()); + assertTrue(handle.getPositionX() < dragToX - 100); + } + @Override protected void tearDown() throws Exception { super.tearDown(); @@ -152,10 +266,24 @@ public class InsertionHandleTest extends ContentShellTestBase { TouchUtils.singleClickView(getInstrumentation(), getContentView(), INITIAL_CLICK_X, INITIAL_CLICK_Y); assertTrue(waitForHandleShowingEquals(true)); - assertTrue(waitForZoomFinished()); + assertWaitForZoomFinished(); + } + + private void assertWaitForZoomFinished() throws Throwable { + // If the polling interval is too short, slowly zooming may be detected as not zooming. + final int POLLING_INTERVAL = 200; + assertTrue(CriteriaHelper.pollForCriteria(new Criteria() { + float mScale = -1; + @Override + public boolean isSatisfied() { + float lastScale = mScale; + mScale = getContentView().getScale(); + return mScale == lastScale; + } + }, CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL, POLLING_INTERVAL)); } - private boolean waitForZoomFinished() throws Throwable { + private boolean waitForHandleViewStopped() throws Throwable { // If the polling interval is too short, slowly zooming may be detected as not zooming. final int POLLING_INTERVAL = 200; return CriteriaHelper.pollForCriteria(new Criteria() { @@ -170,7 +298,8 @@ public class InsertionHandleTest extends ContentShellTestBase { HandleView handle = ihc.getHandleViewForTest(); mPositionX = handle.getPositionX(); mPositionY = handle.getPositionY(); - return mPositionX == lastPositionX && mPositionY == lastPositionY; + return !handle.isDragging() && + mPositionX == lastPositionX && mPositionY == lastPositionY; } }, CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL, POLLING_INTERVAL); } diff --git a/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchUtils.java b/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchUtils.java index 48274a7..5983d73 100644 --- a/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchUtils.java +++ b/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TouchUtils.java @@ -26,7 +26,7 @@ public class TouchUtils extends android.test.TouchUtils { * @param y Relative y location. * @return the absolute x and y location in an array. */ - private static int[] getAbsoluteLocationFromRelative(View v, int x, int y) { + public static int[] getAbsoluteLocationFromRelative(View v, int x, int y) { int location[] = new int[2]; v.getLocationOnScreen(location); location[0] += x; diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc index 21cbd77..02aa00f 100644 --- a/content/renderer/render_view_impl.cc +++ b/content/renderer/render_view_impl.cc @@ -948,6 +948,7 @@ bool RenderViewImpl::OnMessageReceived(const IPC::Message& message) { IPC_MESSAGE_HANDLER(ViewMsg_ExtendSelectionAndDelete, OnExtendSelectionAndDelete) IPC_MESSAGE_HANDLER(ViewMsg_SelectRange, OnSelectRange) + IPC_MESSAGE_HANDLER(ViewMsg_MoveCaret, OnMoveCaret) IPC_MESSAGE_HANDLER(ViewMsg_CopyImageAt, OnCopyImageAt) IPC_MESSAGE_HANDLER(ViewMsg_ExecuteEditCommand, OnExecuteEditCommand) IPC_MESSAGE_HANDLER(ViewMsg_Find, OnFind) @@ -1406,6 +1407,15 @@ void RenderViewImpl::OnSelectRange(const gfx::Point& start, handling_select_range_ = false; } +void RenderViewImpl::OnMoveCaret(const gfx::Point& point) { + if (!webview()) + return; + + Send(new ViewHostMsg_MoveCaret_ACK(routing_id_)); + + webview()->focusedFrame()->moveCaretSelectionTowardsWindowPoint(point); +} + void RenderViewImpl::OnSetHistoryLengthAndPrune(int history_length, int32 minimum_page_id) { DCHECK_GE(history_length, 0); diff --git a/content/renderer/render_view_impl.h b/content/renderer/render_view_impl.h index abe9b57cf..2780f67 100644 --- a/content/renderer/render_view_impl.h +++ b/content/renderer/render_view_impl.h @@ -982,6 +982,7 @@ class CONTENT_EXPORT RenderViewImpl bool notify_result); void OnSelectAll(); void OnSelectRange(const gfx::Point& start, const gfx::Point& end); + void OnMoveCaret(const gfx::Point& point); void OnSetAccessibilityMode(AccessibilityMode new_mode); void OnSetActive(bool active); void OnSetAltErrorPageURL(const GURL& gurl); diff --git a/content/test/data/android/device_files/insertion_handle/input_text.html b/content/test/data/android/device_files/insertion_handle/input_text.html new file mode 100644 index 0000000..772d312 --- /dev/null +++ b/content/test/data/android/device_files/insertion_handle/input_text.html @@ -0,0 +1,7 @@ + + + Test file for input-text-related tests. + + + + -- cgit v1.1