New issue
Advanced search Search tips

Issue 736330 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

Two context menus are displayed on long press on links on iOS 11 device

Project Member Reported by vbarig...@chromium.org, Jun 23 2017

Issue description

App Version: 60.0.3112.42 beta
iOS Version: 11 beta 2
Device: iPads only
URL: nytimes.com, youtube.com

Steps to reproduce:
  1.  Launch chrome.
  2.  Open URL : nytimes.com
  3.  Long press on any of the links for 2 second and release.

Observed results:
Two context menus are displayed.

Expected results:
Only 1 context menu should be displayed.

Number of times you were able to reproduce: 5/5
Bug reproducible after clean install: Yes
Bug reproducible after clearing cache and cookies: Yes
Bug reproducible on Chrome Mobile on Android: Not tested
Bug reproducible on Safari/Firefox: Firefox: Yes, Safari: No
Bug reproducible on current stable build (App Version, iOS Version): Yes on M59
Bug reproducible on the current beta channel build (App Version, iOS Version): Yes

Link to video/image: https://drive.google.com/a/google.com/file/d/0Bz2uwV55gGwDcEllNkNnbnBZV1E/view?usp=sharing
 
Cc: justincohen@chromium.org

Comment 2 by sczs@chromium.org, Jun 28 2017

Cc: -justincohen@chromium.org
Owner: justincohen@chromium.org
Status: Assigned (was: Untriaged)
Cc: justincohen@chromium.org
Components: Mobile>WebView>Glue
Labels: ReleaseBlock-Stable M-61
Owner: michaeldo@chromium.org
Sergio, could you please update Component when you assigning the bug. Thanks!

Comment 4 by cma...@chromium.org, Jul 18 2017

michaeldo@ any update here?

Comment 5 by pkl@chromium.org, Jul 21 2017

Status: Started (was: Assigned)
Looks like michaeldo@ started working on this already: http://crrev/c/579585/
Changing Status.
Cc: mard...@chromium.org eugene...@chromium.org
Sorry, pkl is right, I had started but forgot to update here.

As can be seen in the first upload on that CL, originally I tried to fix this by updating the timeout, however that would make the failure rate in crbug.com/542933 worse by shortening the time Chrome has to fetch the element details from the DOM.

eugenebut@ and I talked about our options and the new patch (on the same CL) actually waits for the DOM element details so we can use whatever long press duration we want.

The side effect (which we believe is a positive one) is that the WKWebView "Safari" context menu will never be shown. Right now it is shown if we can't fetch the details fast enough and it is shown if you long press on context inside of an iframe.

Adding mardini@ to review the last point where behavior changes slightly.
Cc: pinkerton@chromium.org
This change in behaviour sounds fine to me. It's better than show two context menus. Also, just to confirm, these changes apply only to links, correct? Long-pressing regular text would still select it and show [Copy | Look Up | Share]. 

Adding Pink as FYI.
Sorry for the back and forth, but Eugene pointed me to an old hack that will let us fix this in a simpler way without changing the behavior of the menu.

(I still plan to change the behavior in a separate CL in order to reduce the failure rate of the existing context menu and in order to prevent the default WKWebView context menu from showing, but it hopefully won't need to be merged to fix this RBS.)
FYI, with these steps to reproduce only System Context Menu show up:
  1. Launch app
  2. Navigate to the Google.com 
  3. Search for images
  4. Tap on any images tab 
  5. Long Tap on any images 
  6. Tap on Share Mail or Share Notepad

Labels: -Restrict-View-Google
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3700a0865467ca4bcd08aa62f387cb4c597c2e37

commit 3700a0865467ca4bcd08aa62f387cb4c597c2e37
Author: Mike Dougherty <michaeldo@chromium.org>
Date: Tue Aug 01 17:31:29 2017

Show the Chrome context menu when running on iOS 11.

Bring back an old hack to prioritize our gesture recognizer over
WKWebView's when running on iOS 11 or later. The hack was previously
removed in
https://codereview.chromium.org/2627093003/patch/160001/170008.

Bug:  736330 ,  736480 
Change-Id: I669a3d530942e39db246f9b298a5f7a797e9754b
Reviewed-on: https://chromium-review.googlesource.com/585672
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491040}
[modify] https://crrev.com/3700a0865467ca4bcd08aa62f387cb4c597c2e37/ios/web/web_state/ui/crw_context_menu_controller.mm

Status: Fixed (was: Started)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-61; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-61 label, otherwise remove Merge-TBD label. Thanks.
Labels: Merge-Request-61
Project Member

Comment 15 by sheriffbot@chromium.org, Aug 4 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: Less than 28 days to go before AppStore submit on M61
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Verified (was: Fixed)
Verified in canary 62.0.3179.0 on iPad Air(iOS 11 beta4)

Followed steps from comment#0, single context menu is displayed on long press. 
Labels: -Hotlist-Merge-Review -Merge-TBD -Merge-Review-61 Merge-Approved-61
Project Member

Comment 18 by bugdroid1@chromium.org, Aug 9 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7d14fa398e0574893141b8a64b88b53a0664273b

commit 7d14fa398e0574893141b8a64b88b53a0664273b
Author: Mike Dougherty <michaeldo@chromium.org>
Date: Wed Aug 09 18:05:06 2017

Show the Chrome context menu when running on iOS 11.

Bring back an old hack to prioritize our gesture recognizer over
WKWebView's when running on iOS 11 or later. The hack was previously
removed in
https://codereview.chromium.org/2627093003/patch/160001/170008.

TBR=michaeldo@chromium.org

(cherry picked from commit 3700a0865467ca4bcd08aa62f387cb4c597c2e37)

Bug:  736330 ,  736480 
Change-Id: I669a3d530942e39db246f9b298a5f7a797e9754b
Reviewed-on: https://chromium-review.googlesource.com/585672
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#491040}
Reviewed-on: https://chromium-review.googlesource.com/608838
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#405}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/7d14fa398e0574893141b8a64b88b53a0664273b/ios/web/web_state/ui/crw_context_menu_controller.mm

Verified in 61.0.3163.47 on iPad Air(iOS 11 beta6)

Followed steps from comment#0, single context menu is displayed on long press. 

Sign in to add a comment