New issue
Advanced search Search tips

Issue 774196 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Regression
M63



Sign in to add a comment

TouchSelection inside a (BrowserTag) WebView leads to two context menus.

Project Member Reported by wjmaclean@chromium.org, Oct 12 2017

Issue description

Chrome Version: 63.0.3238.0 (Developer Build) (64-bit)
OS: Linux (but probably others?)

Pre-req: a computer with a touch screen.

What steps will reproduce the problem?
(1) Install browser sample.
    https://chrome.google.com/webstore/detail/browser-sample/edggnmnajhcbhlnpjnogkjpghaikidaa?utm_source=chrome-app-launcher-info-dialog
(other webview apps will work too, but this is an easy one to get)

(2) Load a page with selectable text.
(3) Use a touch longpress to select text on the page.

What is the expected result?

Selection handles and the quickmenu should appear.

What happens instead?

Selection handles and quickmenu appear, but a second, differently-themed "copy" menu appears, and the correct quickmenu is non-functional.

Note: this does not occur for OOPIF subframes, just WebView content, so it may be related to the WebView having a different WebContents than the main frame.

See attached photos.

Bisect:

You are probably looking for a change made after 476352 (known good), but no later than 476356 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/c3cf4c7d10c4796ef841fc181d0207cfdc4ad252..27b6f79ce778bb7f2bc9a15a27e0d86d6f1656e8

Likely suspect: https://codereview.chromium.org/2785853002
 
IMG_20171012_145442_NoExif.jpg
6.9 MB View Download
IMG_20171012_145435_NoExif.jpg
5.4 MB View Download
I'm suspicious of this part of the suspect CL:

https://codereview.chromium.org/2785853002/diff/700001/third_party/WebKit/Source/web/WebViewImpl.cpp

BrowserTag WebViews are unlike OOPIFs in that they have *two* WebViewImpls ... one for the guest content and one for the embedder content. In an oopif this gesture event may instead be handled by WebFrameWidgetBase/Impl (not sure which).
Labels: -Type-Bug Type-Bug-Regression
Labels: M63 ReleaseBlock-Beta
I don't know if it's possible to get this fixed for the M62 stable release, but it would be *great* if we could do that.
I think the problem is with how BrowserTag WebViews are dealing with context menu events. If you long-press an empty input/textarea the expected result is that the touch handle shows below the selection caret and above the caret there is a floating quick-menu with items "Paste, ...". On WebView when we long-press on an empty input there is no quick-menu and instead there is a drop-down menu (one you'd get from right clicking). This leads me to believe that TouchSelectionControllerClientAura::HandleContextMenu() is not getting called. This would also explain this bug. In my CL I expected the long-press context menu event to be handled by TouchSelectionControllerClientAura::HandleContextMenu() and not be propagated thus not showing the dropdown menu. This was done with this change: https://codereview.chromium.org/2785853002/diff/700001/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc.

Any update on this? Branch date for M64 is rapidly approaching.
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 8 2018

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

commit 55cc4c00b301ab442b15d9c273c4228854b513dd
Author: Pedro Amaral <amaralp@chromium.org>
Date: Mon Jan 08 21:52:57 2018

Don't show touch selection Context Menus in guest view

Normally we don't show a context menu on touch selections (see
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc?l=186). This check isn't done for guest
webviews and it causes the linked bug. This CL adds this check for guest
webviews.

Bug:  774196 
Change-Id: I2ea827339d5782850fb626e7c82c282f7aac9308
Reviewed-on: https://chromium-review.googlesource.com/719474
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Commit-Queue: Pedro Amaral <amaralp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527770}
[modify] https://crrev.com/55cc4c00b301ab442b15d9c273c4228854b513dd/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
[modify] https://crrev.com/55cc4c00b301ab442b15d9c273c4228854b513dd/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc
[modify] https://crrev.com/55cc4c00b301ab442b15d9c273c4228854b513dd/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/55cc4c00b301ab442b15d9c273c4228854b513dd/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/55cc4c00b301ab442b15d9c273c4228854b513dd/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/55cc4c00b301ab442b15d9c273c4228854b513dd/content/public/test/browser_test_utils.h

Status: Fixed (was: Assigned)

Sign in to add a comment