Chromium Code Reviews
Help | Chromium Project | Sign in
(1)

Issue 2785853002: Selection Action mode triggered like a context menu

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month ago by amaralp
Modified:
5 days, 8 hours ago
CC:
agrieve+watch_chromium.org, android-webview-reviews_chromium.org, blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, dglazkov+blink, dtapuska+blinkwatch_chromium.org, dtapuska+chromiumwatch_chromium.org, jam, jochen+watch_chromium.org, kinuko+watch, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, nona+watch_chromium.org, Navid Zolghadr, Peter Beverloo, shuchen+watch_chromium.org, James Su, yusukes+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Selection Action mode triggered like a context menu Currently the selection action mode is triggered by selection events (SELECTION_HANDLES_SHOWN). The goal of this CL is to make the mode be triggered by context menu events. This is beneficial for two reasons: 1) The paste popup menu is already being triggered by context menu events and we'd like to eventually unify the selection and paste menus. 2) Currently the selection action mode uses state from the IME which can lead to race conditions causing the menu to show stale information. Using context menu events fixes this problem because we can pass all relevant state through |ContextMenuParams|. BUG=627234

Patch Set 1 #

Patch Set 2 : Fixed touch emulator unittest #

Patch Set 3 : updating layout #

Patch Set 4 : rebase #

Patch Set 5 : fixing aura #

Patch Set 6 : forgot the not #

Patch Set 7 : Fixing select all and ime #

Patch Set 8 : fixed some bugs #

Patch Set 9 : formatting #

Patch Set 10 : rebase #

Patch Set 11 : fixes failing tests #

Patch Set 12 : moved contextmenuallowedscope #

Patch Set 13 : actually added ContextMenuAllowedScope #

Patch Set 14 : fixing include and core_export #

Patch Set 15 : Random fixes #

Patch Set 16 : Rebased #

Patch Set 17 : Another rebase #

Patch Set 18 : Changed Smart Selection #

Patch Set 19 : Rebase #

Patch Set 20 : Fixing rebase bug #

Total comments: 13
Unified diffs Side-by-side diffs Delta from patch set Stats (+193 lines, -114 lines) Patch
M android_webview/native/aw_web_contents_view_delegate.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/android/tab_contents/chrome_web_contents_view_delegate_android.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/android/content_view_core_impl.h View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M content/browser/android/content_view_core_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 3 chunks +18 lines, -5 lines 1 comment Download
M content/browser/renderer_host/input/touch_emulator_client.h View 1 chunk +2 lines, -1 line 2 comments Download
M content/browser/renderer_host/input/touch_emulator_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/renderer_host/input/touch_selection_controller_client_aura.cc View 1 2 3 4 5 1 chunk +6 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +5 lines, -3 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +1 line, -1 line 0 comments Download
M content/common/frame_messages.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +2 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java View 1 2 3 4 5 6 7 8 9 1 chunk +5 lines, -2 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/ContextSelectionClient.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +2 lines, -14 lines 1 comment Download
M content/public/android/java/src/org/chromium/content/browser/SelectionClient.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -1 line 1 comment Download
M content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 6 chunks +31 lines, -19 lines 3 comments Download
M content/public/browser/android/content_view_core.h View 1 chunk +1 line, -1 line 0 comments Download
M content/public/common/context_menu_params.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +5 lines, -0 lines 3 comments Download
M content/public/common/context_menu_params.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/context_menu_params_builder.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/render_frame_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +4 lines, -0 lines 0 comments Download
M content/shell/browser/shell_web_contents_view_delegate_android.cc View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +3 lines, -0 lines 1 comment Download
M third_party/WebKit/Source/core/editing/FrameSelection.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/input/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
A + third_party/WebKit/Source/core/input/ContextMenuAllowedScope.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +2 lines, -1 line 0 comments Download
A + third_party/WebKit/Source/core/input/ContextMenuAllowedScope.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/input/EventHandler.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/input/EventHandler.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +56 lines, -0 lines 1 comment Download
M third_party/WebKit/Source/core/input/GestureManager.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +0 lines, -2 lines 0 comments Download
D third_party/WebKit/Source/web/ContextMenuAllowedScope.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -24 lines 0 comments Download
D third_party/WebKit/Source/web/ContextMenuAllowedScope.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -23 lines 0 comments Download
M third_party/WebKit/Source/web/ContextMenuClientImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 4 chunks +9 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/web/WebViewImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 3 chunks +8 lines, -4 lines 0 comments Download
M third_party/WebKit/public/web/WebContextMenuData.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 3 chunks +7 lines, -1 line 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 86 (81 generated)
amaralp
PTAL
1 week, 1 day ago (2017-04-21 05:18:42 UTC) #80
boliu
On 2017/04/21 05:18:42, amaralp wrote: > PTAL For CL with many reviewers, you should specify ...
1 week ago (2017-04-21 18:50:16 UTC) #83
boliu
https://codereview.chromium.org/2785853002/diff/380001/content/browser/renderer_host/input/touch_emulator_client.h File content/browser/renderer_host/input/touch_emulator_client.h (right): https://codereview.chromium.org/2785853002/diff/380001/content/browser/renderer_host/input/touch_emulator_client.h#newcode25 content/browser/renderer_host/input/touch_emulator_client.h:25: bool from_touch = false) = 0; default arg on ...
1 week ago (2017-04-21 21:20:19 UTC) #84
aelias
Nice cleanup overall. https://codereview.chromium.org/2785853002/diff/380001/content/browser/android/content_view_core_impl.cc File content/browser/android/content_view_core_impl.cc (right): https://codereview.chromium.org/2785853002/diff/380001/content/browser/android/content_view_core_impl.cc#newcode627 content/browser/android/content_view_core_impl.cc:627: params.from_touch; Is it possible to use ...
1 week ago (2017-04-21 21:42:19 UTC) #85
Jinsuk Kim
5 days, 8 hours ago (2017-04-24 06:33:16 UTC) #86
Nice refactoring.

FYI I'm working on https://crrev.com/2826303003 not to use content_view_core for
selection events. Would you leave a TODO somewhere for a reminder later for
further refactoring?

https://codereview.chromium.org/2785853002/diff/380001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/SelectionClient.java
(right):

https://codereview.chromium.org/2785853002/diff/380001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/SelectionClient.java:37:
boolean sendsSelectionPopupUpdates(boolean shouldSuggest);
Update javadoc.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46