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

Issue 764192 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 2
Type: Bug



Sign in to add a comment

Extension Menu Will be Covered by Other Extension in Right-Click Context Menu

Reported by qsw...@gmail.com, Sep 12 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.9 Safari/537.36

Steps to reproduce the problem:
We use 3 extensions as example.
Ex1: https://goo.gl/UVQ3NK
Ex2: https://goo.gl/1kAJAT
Ex3: https://goo.gl/UVQ3NK
And a webpage include picture.

1. Install Ex1, and right-click in a picture on the web, observe right-click context menu
2. Install Ex2, and do the same observe action
3. Install Ex3, and do the same observe action

What is the expected behavior?
All of the extension their own menu in the right-click context menu.
This is what we can see in Stable version.

What went wrong?
There is only one menu location for extension.
After install Ex2, Ex1's menu will be covered.
After install Ex3, Ex2's menu will be covered.
This cover order will not be influenced by enable order. Ex3 always covers other extensions' menu.

And also for some other extensions.
The attached images show what different between Stable and Dev when all of the example extensions are enabled.

Did this work before? N/A 

Chrome version: 62.0.3202.9  Channel: dev
OS Version: 10.0
Flash Version: 

What confuse me is that some extensions always has their own menu, no matter what other extensions enabled at the same time. But other extensions's menu will be covered.
It seems like this Dev version set a limit for extension, give only one menu location for "normal" extensions. This will cause illusion for users about the extensions don't work anymore.
It should be mend, I think.
 
Stable.png
4.2 KB View Download
Dev.png
2.9 KB View Download

Comment 1 by woxxom@gmail.com, Sep 12 2017

Ex3 link above is incorrect, it's the same as Ex1.
The correct Ex3 link is https://chrome.google.com/webstore/detail/image-search-options/kljmejbpilkadikecejccebmccagifhl

Repro:
1. install 3 extensions using Ex1, Ex2 links in c#0 and Ex3 link in this comment
2. open https://www.google.com/intl/en/chrome/browser/welcome.html
3. right-click Chrome logo in the top-left corner

Expected: all 3 context menu items added by extensions are visible
Observed: only 1 or 2 menu items are shown

Bisect info: 496204 (good) - 496213 (bad)
https://chromium.googlesource.com/chromium/src/+log/e95af5e5..d5a78f8b?pretty=fuller
Suspecting r496210 "[Extensions] Added visibility property to chrome.contextMenu items API"
Landed in 62.0.3194.0

P.S. The suspected CL checks for "visibility" property differently in context_menus_api_helpers.h [1][2], might be the reason of the bug.
  [1]: https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/context_menus/context_menus_api_helpers.h?l=123&rcl=44248851c5f19227c3fc9bcc8c60285175064a3a
  [2]: https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/context_menus/context_menus_api_helpers.h?l=228&rcl=44248851c5f19227c3fc9bcc8c60285175064a3a
Labels: Needs-Bisect Needs-Triage-M62

Comment 3 by qsw...@gmail.com, Sep 13 2017

Erratum: The links provided in c#0 has iterative items.
To retro the given situation, the three extension should be:

Ex1: https://goo.gl/Q1qG7a
Ex2: https://goo.gl/1kAJAT
Ex3: https://goo.gl/UVQ3NK

Image Search Option is the one always be covered in my end.
Labels: -Needs-Bisect hasbisect-per-revision ReleaseBlock-Stable Triaged-ET M-62 Needs-Milestone OS-Linux
Owner: catmulli...@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce on 62.0.3202.9, latest stable 61.0.3163.79, Latest Canary 63.0.3213.3 with the mentioned steps on Ubuntu 14.04 and windows 7
Note: Issue is not reproducible on Mac

Below is the Bisect Information:

Good build: 62.0.3193.0
Bad Build : 62.0.3194.0

You are probably looking for a change made after 496209 (known good), but no later than 496210 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
https://chromium.googlesource.com/chromium/src/+log/dab1034335c0d82332e0f7572ba9781637d2e6af..44248851c5f19227c3fc9bcc8c60285175064a3a

From the above change log Suspecting: https://chromium-review.googlesource.com/553578

@Catherine Mullings - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 15 2017

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

commit 783d51fa63399dc0c7022ff06a73b2448e5632e8
Author: Catherine Mullings <catmullings@chromium.org>
Date: Fri Sep 15 01:50:22 2017

Extensions: Fix context menu to show multiple top-level extension menus

 crbug.com/764192  reports a regression in which only 1 of many top-level
extension-named menu items are displayed in the context menu. This bug
is namely targetted at extension-named menu items that have child items.

This CL fixes this regression such that *all* top-level extension-named
menu items are displayed in the context menu.

Bug:  764192 
Change-Id: Ibac5707b3fb5416242a3e408feab579b2553b57c
Reviewed-on: https://chromium-review.googlesource.com/667950
Commit-Queue: catmullings <catmullings@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502129}
[modify] https://crrev.com/783d51fa63399dc0c7022ff06a73b2448e5632e8/chrome/browser/extensions/api/context_menus/context_menu_apitest.cc
[modify] https://crrev.com/783d51fa63399dc0c7022ff06a73b2448e5632e8/chrome/browser/extensions/context_menu_matcher.cc
[add] https://crrev.com/783d51fa63399dc0c7022ff06a73b2448e5632e8/chrome/test/data/extensions/api_test/context_menus/simple/one/background.js
[add] https://crrev.com/783d51fa63399dc0c7022ff06a73b2448e5632e8/chrome/test/data/extensions/api_test/context_menus/simple/one/manifest.json
[add] https://crrev.com/783d51fa63399dc0c7022ff06a73b2448e5632e8/chrome/test/data/extensions/api_test/context_menus/simple/two/background.js
[add] https://crrev.com/783d51fa63399dc0c7022ff06a73b2448e5632e8/chrome/test/data/extensions/api_test/context_menus/simple/two/manifest.json

Status: Fixed (was: Started)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-62; 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-62 label, otherwise remove Merge-TBD label. Thanks.
Cc: rdevlin....@chromium.org
Labels: -Merge-TBD Merge-Request-62
Project Member

Comment 10 by sheriffbot@chromium.org, Sep 16 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-62 Merge-Approved-62
Thanks for the fix - approving merge to M62. Branch:3202
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 18 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d87faea6fdb1c8c38e2e644a30feea5d4fbdd26a

commit d87faea6fdb1c8c38e2e644a30feea5d4fbdd26a
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Mon Sep 18 18:49:27 2017

Extensions: Fix context menu to show multiple top-level extension menus

 crbug.com/764192  reports a regression in which only 1 of many top-level
extension-named menu items are displayed in the context menu. This bug
is namely targetted at extension-named menu items that have child items.

This CL fixes this regression such that *all* top-level extension-named
menu items are displayed in the context menu.

TBR=catmullings@chromium.org

(cherry picked from commit 783d51fa63399dc0c7022ff06a73b2448e5632e8)

Bug:  764192 
Change-Id: Ibac5707b3fb5416242a3e408feab579b2553b57c
Reviewed-on: https://chromium-review.googlesource.com/667950
Commit-Queue: catmullings <catmullings@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#502129}
Reviewed-on: https://chromium-review.googlesource.com/671683
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#298}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/d87faea6fdb1c8c38e2e644a30feea5d4fbdd26a/chrome/browser/extensions/api/context_menus/context_menu_apitest.cc
[modify] https://crrev.com/d87faea6fdb1c8c38e2e644a30feea5d4fbdd26a/chrome/browser/extensions/context_menu_matcher.cc
[add] https://crrev.com/d87faea6fdb1c8c38e2e644a30feea5d4fbdd26a/chrome/test/data/extensions/api_test/context_menus/simple/one/background.js
[add] https://crrev.com/d87faea6fdb1c8c38e2e644a30feea5d4fbdd26a/chrome/test/data/extensions/api_test/context_menus/simple/one/manifest.json
[add] https://crrev.com/d87faea6fdb1c8c38e2e644a30feea5d4fbdd26a/chrome/test/data/extensions/api_test/context_menus/simple/two/background.js
[add] https://crrev.com/d87faea6fdb1c8c38e2e644a30feea5d4fbdd26a/chrome/test/data/extensions/api_test/context_menus/simple/two/manifest.json

Sign in to add a comment