Context menu separators missing from Web Extensions
Reported by
ibrahim....@gmail.com,
Nov 11
|
|||||||||||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.102 Safari/537.36
Steps to reproduce the problem:
Separator lines, created with chrome.contextMenus.create({"type": "separator"}); are not being displayed anymore.
Neither with "id" and "contexts" attributes specified: chrome.contextMenus.create({"type": "separator", "id": "sep1", contexts: ["all"]});
What is the expected behavior?
On older versions of Chrome (up to 69) it shows a separator line.
What went wrong?
On latest version (70), it shows nothing.
Did this work before? N/A
Does this work in other browsers? N/A
Chrome version: 70.0.3538.102 Channel: stable
OS Version: OS X 10.14.1
Flash Version:
Tested with these two versions:
69.0.3497.92 - separator works
70.0.3538.102 - doesn't work
,
Nov 12
Thanks for the issue...
As per comment #0, Tried to reproduce the issue on reported chrome 70.0.3538.102 using Mac 10.14.0.
Steps:
-----
1. Launched chrome
2. Opened Dev tools> Entered given code on console >chrome.contextMenus.create({"type": "separator"}); and chrome.contextMenus.create({"type": "separator", "id": "sep1", contexts: ["all"]});
As we are getting uncaught error on console.
@Reporter : It would be really helpful if a sample URL/Test file is provided, so that we can investigate the issue further.
Thanks.!
,
Nov 12
Using "Context Menus Sample" extension from the official gallery: https://developer.chrome.com/extensions/samples Bisected to r584789 = 25ea144293b24ee65410a18c44ea1b79a42547e9 = https://crrev.com/c/1181770 by newcomer@chromium.org "Reland "cros: Enable touchable app context menus by default."" Landed in 70.0.3530.0 Confirmed by disabling the CL via command line and observing the bug is gone: chrome --disable-features=EnableTouchableAppContextMenu
,
Nov 12
Thanks for the report - I'll assign this to newcomer@ per #3.
,
Nov 12
I'll target M-73 because this is a P-2, and I won't have time for p-2's for a few weeks. If this is too slow of a fix, LMK. There is some shared code to generate extension menu items between the CrOS System UI and mac. I have a pretty good idea on where to look.
,
Nov 15
FYI: Chrome v70 in Windows 10 is also affected. Beside the itemType separators also the separator between the native Chrome functions and web extensions is gone. From my unfortunately duplicated bug report today a screenshot how it looked before and after. May this helps. A faster fix than M73 would be welcomed because it really messes with the usability of context search menus.
,
Nov 15
Issue 905686 has been merged into this issue.
,
Nov 15
In that case, I'll unnasign myself so someone else can pick it up while I'm busy. Re prioritizing and bringing the priority back to M-72.
,
Nov 15
s/priority/milestone
,
Nov 15
My guess is that this[1] class is used by Mac to generate the extension menus. We should check if we are running chrome on mac here, and include the separators. This would need to happen for RecursivelyAppendExtensionItems and AppendExtensionItems. [1] https://cs.chromium.org/chromium/src/chrome/browser/extensions/context_menu_matcher.cc?l=87
,
Dec 5
Picking this back up.
,
Dec 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1ee2d56d1d898cbda771ca8e5a1d790ed76d684f commit 1ee2d56d1d898cbda771ca8e5a1d790ed76d684f Author: Alex Newcomer <newcomer@chromium.org> Date: Thu Dec 06 02:05:05 2018 cros: Remove touchable flag from context_menu_matcher.cc This feature is enabled by default, so remove the flag. This feature was accidentally breaking menu separators on other platforms, so re-enable them on !OS_CHROMEOS. Bug: 904220 Change-Id: Ief676d4fabdbe800214b5dd441a7fbfb966398b9 Reviewed-on: https://chromium-review.googlesource.com/c/1364066 Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Alex Newcomer <newcomer@chromium.org> Cr-Commit-Position: refs/heads/master@{#614232} [modify] https://crrev.com/1ee2d56d1d898cbda771ca8e5a1d790ed76d684f/chrome/browser/extensions/context_menu_matcher.cc [modify] https://crrev.com/1ee2d56d1d898cbda771ca8e5a1d790ed76d684f/chrome/browser/extensions/extension_context_menu_browsertest.cc
,
Dec 6
,
Dec 7
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/342583349feb5ef68fd1fcfc95dcdac19fc65531 commit 342583349feb5ef68fd1fcfc95dcdac19fc65531 Author: Alex Newcomer <newcomer@chromium.org> Date: Fri Dec 07 18:59:09 2018 cros: Remove touchable flag from context_menu_matcher.cc This feature is enabled by default, so remove the flag. This feature was accidentally breaking menu separators on other platforms, so re-enable them on !OS_CHROMEOS. Bug: 904220 Change-Id: Ief676d4fabdbe800214b5dd441a7fbfb966398b9 Reviewed-on: https://chromium-review.googlesource.com/c/1364066 Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Alex Newcomer <newcomer@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#614232}(cherry picked from commit 1ee2d56d1d898cbda771ca8e5a1d790ed76d684f) Reviewed-on: https://chromium-review.googlesource.com/c/1368277 Reviewed-by: Alex Newcomer <newcomer@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#144} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/342583349feb5ef68fd1fcfc95dcdac19fc65531/chrome/browser/extensions/context_menu_matcher.cc [modify] https://crrev.com/342583349feb5ef68fd1fcfc95dcdac19fc65531/chrome/browser/extensions/extension_context_menu_browsertest.cc
,
Dec 7
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/342583349feb5ef68fd1fcfc95dcdac19fc65531 Commit: 342583349feb5ef68fd1fcfc95dcdac19fc65531 Author: newcomer@chromium.org Commiter: newcomer@chromium.org Date: 2018-12-07 18:59:09 +0000 UTC cros: Remove touchable flag from context_menu_matcher.cc This feature is enabled by default, so remove the flag. This feature was accidentally breaking menu separators on other platforms, so re-enable them on !OS_CHROMEOS. Bug: 904220 Change-Id: Ief676d4fabdbe800214b5dd441a7fbfb966398b9 Reviewed-on: https://chromium-review.googlesource.com/c/1364066 Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Alex Newcomer <newcomer@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#614232}(cherry picked from commit 1ee2d56d1d898cbda771ca8e5a1d790ed76d684f) Reviewed-on: https://chromium-review.googlesource.com/c/1368277 Reviewed-by: Alex Newcomer <newcomer@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#144} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by phanindra.mandapaka@chromium.org
, Nov 11