New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 6 users
Status: Fixed
Owner:
Closed: Jun 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocked on:
issue 633281
issue 733867

Blocking:
issue 615435
issue 626830
issue 707857
issue 725003



Sign in to add a comment
Refactor Clank Context Menu
Project Member Reported by amaralp@chromium.org, Jul 11 2016 Back to list
Issue to track progress on refactoring.

Currently ContentViewCore.java triggers the "cut, copy, paste" menu. This refactor aims to have blink trigger the menu.
 
Components: -Blink UI>Browser
Comment 2 by mustaq@chromium.org, Jul 12 2016
Blocking: 468806
Blockedon: 633281
Comment 4 by mustaq@chromium.org, Nov 11 2016
Blocking: -468806
The mouse event path has been fixed w/o this.
Blocking: 626830
Blocking: 615435
Project Member Comment 8 by bugdroid1@chromium.org, Mar 2 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f88569b6a113509e7e7245ca297221701d07d4a3

commit f88569b6a113509e7e7245ca297221701d07d4a3
Author: amaralp <amaralp@chromium.org>
Date: Thu Mar 02 07:28:26 2017

Make SelectionPopupController.ShowPastePopup only be triggered by Blink

This patch is progress towards the goal of having Blink triggering all
of clank's SelectActionMode context menus. In particular this patch
has Blink trigger the menu when an insertion handle is tapped or
dragged.

BUG= 627234 

Review-Url: https://codereview.chromium.org/2721813002
Cr-Commit-Position: refs/heads/master@{#454203}

[modify] https://crrev.com/f88569b6a113509e7e7245ca297221701d07d4a3/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/f88569b6a113509e7e7245ca297221701d07d4a3/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/f88569b6a113509e7e7245ca297221701d07d4a3/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/f88569b6a113509e7e7245ca297221701d07d4a3/content/browser/web_contents/web_contents_android.h
[modify] https://crrev.com/f88569b6a113509e7e7245ca297221701d07d4a3/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/f88569b6a113509e7e7245ca297221701d07d4a3/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
[modify] https://crrev.com/f88569b6a113509e7e7245ca297221701d07d4a3/content/public/android/java/src/org/chromium/content_public/browser/WebContents.java

Project Member Comment 9 by bugdroid1@chromium.org, Mar 2 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a5687419f74d3767023162b603ac08b2a998dc6

commit 7a5687419f74d3767023162b603ac08b2a998dc6
Author: amaralp <amaralp@chromium.org>
Date: Thu Mar 02 20:01:24 2017

Revert of Make SelectionPopupController.ShowPastePopup only be triggered by Blink (patchset #3 id:40001 of https://codereview.chromium.org/2721813002/ )

Reason for revert:
Caused flakiness (crbug.com/697934)

Original issue's description:
> Make SelectionPopupController.ShowPastePopup only be triggered by Blink
>
>
> This patch is progress towards the goal of having Blink triggering all
> of clank's SelectActionMode context menus. In particular this patch
> has Blink trigger the menu when an insertion handle is tapped or
> dragged.
>
> BUG= 627234 
>
> Review-Url: https://codereview.chromium.org/2721813002
> Cr-Commit-Position: refs/heads/master@{#454203}
> Committed: https://chromium.googlesource.com/chromium/src/+/f88569b6a113509e7e7245ca297221701d07d4a3

TBR=aelias@chromium.org,boliu@chromium.org,jinsukkim@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 627234 

Review-Url: https://codereview.chromium.org/2727203003
Cr-Commit-Position: refs/heads/master@{#454355}

[modify] https://crrev.com/7a5687419f74d3767023162b603ac08b2a998dc6/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/7a5687419f74d3767023162b603ac08b2a998dc6/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/7a5687419f74d3767023162b603ac08b2a998dc6/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/7a5687419f74d3767023162b603ac08b2a998dc6/content/browser/web_contents/web_contents_android.h
[modify] https://crrev.com/7a5687419f74d3767023162b603ac08b2a998dc6/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/7a5687419f74d3767023162b603ac08b2a998dc6/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
[modify] https://crrev.com/7a5687419f74d3767023162b603ac08b2a998dc6/content/public/android/java/src/org/chromium/content_public/browser/WebContents.java

Project Member Comment 10 by bugdroid1@chromium.org, Mar 21 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c8279c96ec0483bfc86fddd66a860db2dccae18

commit 8c8279c96ec0483bfc86fddd66a860db2dccae18
Author: amaralp <amaralp@chromium.org>
Date: Tue Mar 21 20:05:53 2017

Make SelectionPopupController.ShowPastePopup only be triggered by Blink

This patch is progress towards the goal of having Blink triggering all
of clank's SelectActionMode context menus. In particular this patch
has Blink trigger the menu when an insertion handle is tapped or
dragged.

BUG= 627234 

Review-Url: https://codereview.chromium.org/2721813002
Cr-Original-Commit-Position: refs/heads/master@{#454203}
Committed: https://chromium.googlesource.com/chromium/src/+/f88569b6a113509e7e7245ca297221701d07d4a3
Review-Url: https://codereview.chromium.org/2721813002
Cr-Commit-Position: refs/heads/master@{#458525}

[modify] https://crrev.com/8c8279c96ec0483bfc86fddd66a860db2dccae18/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/8c8279c96ec0483bfc86fddd66a860db2dccae18/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/8c8279c96ec0483bfc86fddd66a860db2dccae18/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/8c8279c96ec0483bfc86fddd66a860db2dccae18/content/browser/web_contents/web_contents_android.h
[modify] https://crrev.com/8c8279c96ec0483bfc86fddd66a860db2dccae18/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/8c8279c96ec0483bfc86fddd66a860db2dccae18/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
[modify] https://crrev.com/8c8279c96ec0483bfc86fddd66a860db2dccae18/content/public/android/java/src/org/chromium/content_public/browser/WebContents.java

Blocking: 707857
Project Member Comment 13 by bugdroid1@chromium.org, May 12 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/354e86bea96bc19c3d922f3733b81a8f8b635993

commit 354e86bea96bc19c3d922f3733b81a8f8b635993
Author: amaralp <amaralp@chromium.org>
Date: Fri May 12 20:37:05 2017

Make Paste Popup use selection rect for positioning

Currently the paste popup and the selection action mode are being positioned
differently. This CL makes it so the paste popup is positioned in the same manner
as the selection action mode.

BUG= 627234 

Review-Url: https://codereview.chromium.org/2855353002
Cr-Commit-Position: refs/heads/master@{#471433}

[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/browser/android/selection_popup_controller.cc
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/browser/android/selection_popup_controller.h
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/common/frame_messages.h
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/public/android/java/src/org/chromium/content/browser/input/FloatingPastePopupMenu.java
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/public/android/java/src/org/chromium/content/browser/input/LegacyPastePopupMenu.java
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/public/android/java/src/org/chromium/content/browser/input/PastePopupMenu.java
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/public/common/context_menu_params.h
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
[modify] https://crrev.com/354e86bea96bc19c3d922f3733b81a8f8b635993/third_party/WebKit/public/web/WebContextMenuData.h

Project Member Comment 15 by bugdroid1@chromium.org, May 25 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d123058c5e6ba14b52241c6d39fbbbeebece1bd

commit 2d123058c5e6ba14b52241c6d39fbbbeebece1bd
Author: amaralp <amaralp@chromium.org>
Date: Thu May 25 19:04:19 2017

Use hasSelection() instead of mHasSelection

We should be using the getter method, hasSelection() instead of accessing
the mHasSelection directly. This will make things simpler in
crrev.com/2785853002 when hasSelection() will do something more
complicated than returning a boolean field.

BUG= 627234 

Review-Url: https://codereview.chromium.org/2904033002
Cr-Commit-Position: refs/heads/master@{#474739}

[modify] https://crrev.com/2d123058c5e6ba14b52241c6d39fbbbeebece1bd/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java

Blocking: 725003
Project Member Comment 17 by bugdroid1@chromium.org, Jun 1
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/58d4d0b2d9511fe57bde3cbe82e486658183456c

commit 58d4d0b2d9511fe57bde3cbe82e486658183456c
Author: amaralp <amaralp@chromium.org>
Date: Thu Jun 01 18:20:23 2017

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|.

In Blink we make range touch selections trigger contextmenu events. This means
that a long-press or double tap that results in a range selection sends a context
menu event. Because of Android O's smart select we also need to know if the
context menu was triggered by moving touch handles or select all so we plumb that
information through to ContextMenuParams.

BUG= 627234 

Review-Url: https://codereview.chromium.org/2785853002
Cr-Commit-Position: refs/heads/master@{#476355}

[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/android_webview/browser/aw_web_contents_view_delegate.cc
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/chrome/browser/ui/android/tab_contents/chrome_web_contents_view_delegate_android.cc
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/browser/renderer_host/input/stylus_text_selector.cc
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/browser/renderer_host/input/stylus_text_selector.h
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/browser/renderer_host/input/stylus_text_selector_unittest.cc
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/child/assert_matching_enums.cc
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/public/browser/android/content_view_core.h
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/content/shell/browser/shell_web_contents_view_delegate_android.cc
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/third_party/WebKit/LayoutTests/fast/events/hit-test-counts-expected.txt
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/third_party/WebKit/LayoutTests/fast/events/hit-test-counts.html
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/third_party/WebKit/LayoutTests/fast/events/mouse-event-buttons-attribute-expected.txt
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/third_party/WebKit/Source/core/editing/FrameSelection.cpp
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/third_party/WebKit/Source/core/editing/SelectionController.cpp
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/third_party/WebKit/Source/core/editing/SelectionController.h
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/third_party/WebKit/Source/core/input/EventHandler.cpp
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/third_party/WebKit/Source/core/input/GestureManager.cpp
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/third_party/WebKit/public/web/WebMenuSourceType.h
[modify] https://crrev.com/58d4d0b2d9511fe57bde3cbe82e486658183456c/ui/base/ui_base_types.h

Labels: Merge-Request-60
Requesting merge of http://crrev.com/476355 to M60 because it's a dependency for http://crbug.com/707857
Project Member Comment 19 by sheriffbot@chromium.org, Jun 1
Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-60 Merge-Rejected-60
This is larger than we should be taking post-branch, and taking a patch like this to enhance a new feature is not sufficient rationale.  Rejected for M-60.
Status: Fixed
OK then, marking fixed since the refactoring is complete.
Blockedon: 733867
Sign in to add a comment