New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 768755 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Context menus on Android no longer have alt text/URL

Project Member Reported by k...@chromium.org, Sep 26 2017

Issue description

I recently heard about  crbug.com/687787  from Ted. This CL has changed all our context menus (including normal long-press) to right click menus which has more significant UX implications than the bug title would suggest. For example, we don't appear to show alt/URL in context menus any more (which is useful for security + accessibility).

Can we restrict the right click menu to only mouse right click?
 

Comment 1 by aelias@chromium.org, Sep 26 2017

What device, Android version and URL is this on, and could you post a screenshot of the bad context menu?

I do observe a UX change on long-press on Nexus 6P/N and Galaxy S8/N in dev channel and up, but I don't see any missing information (I do see URL and alt text).

Comment 2 by cblume@chromium.org, Sep 26 2017

I took a look and the context menus do indeed appear different.

I think this needs two separate fixes.
First, in our Java ContextMenuHelper we no longer have access to the media source (mouse, long press, etc). So I need to pipe the media source from C++ into Java.
Second, the different appearance might mean that we somehow provided a context menu layout which isn't being used in the new API call...or maybe we overrode the function. Whatever the case, we should probably use that same layout in the right click menu.

The net effect of these two changes would be the menu always looking correct and only being positioned by mouse clicks.

Comment 3 by k...@chromium.org, Sep 26 2017

Ah yes - we do have a 50-50 experiment for a new context menu design but that's not launching in this milestone. You need to disable the new context menu under a "enable custom context menu" chrome://flags.

(and sorry - Pixel/Oreo/an AMP stricle. I'm on airplane wifi so screenshots will be more challenging but it should be easy to repro)

Comment 4 by cblume@chromium.org, Sep 26 2017

A part of this is probably the 50-50 experiment.
Attached are images from Android M (using non-positioned context menus) with the new context menu designs. The different appearances are from Dev/Beta.

In addition, I am attaching a screenshot from Android O (which will use positioned context menus). It seems to be using the new design.

So then I set the flag to disabled and you can see the old Context menus which don't show the URL / Alt text.

This is indeed a change from that 50-50 test and not a change from the menu positioning.
Interestingly, the new menu seems to not be using the new position code :D
O-working.png
217 KB View Download
O-old.png
1.8 MB View Download
M-new-no-images.png
274 KB View Download
M-new-images.png
212 KB View Download

Comment 5 by cblume@chromium.org, Sep 28 2017

Issue 768402 has been merged into this issue.
Can we get an update here?

Please note that M63 Beta is nearing.

Comment 7 by cblume@chromium.org, Oct 16 2017

Sure!
I reverted my change and will bring it back with two updates:
* Only position the dialog when the event source was a right mouse click (and leave the long-touch unpositioned)
* Update the 50-50 test to also use the positioning code.

However, something not clear in my screenshots is I don't think my code removed the image/URL. I think it wasn't there previously. You can see the old / new image context menu here: https://www.xda-developers.com/chromes-new-custom-context-menu-revamps-the-linkimagevideo-context-menu/

Comment 8 by aelias@chromium.org, Oct 16 2017

Status: Fixed (was: Assigned)
Marking this fixed since it was reverted.  The reland can be followed up on the original bug.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-63; 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-63 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD
For context, the original bug is here: https://bugs.chromium.org/p/chromium/issues/detail?id=687787

Sign in to add a comment