1704474 - Duplicated commands in context menu for icons in address bar

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[Francesco Lodolo [:flod]]

Attachment: Screenshot

I have two extensions adding an icon to the address bar, and the add/remove commands are duplicated. Tried on a clean en-US profile, and same issue.

macOS, nightly 89 (20210411210108)

Comment 1

[Marco Bonardo [:mak]]

Those entries before the separator and the separator should not even be visible, let alone duplicated.

I assume this is with the proton pref enabled. Does it also happen with proton disabled?

Did you flip on some of the native menus prefs for macOS?
Asking because I don't see this on Windows.

Comment 2

[Francesco Lodolo [:flod]]

Apparently the en-US profile had some modifications too. On a completely clean profile, I only see the last two commands, and the menu is not following the OS theme.

After 20 minutes chasing down differences, looks like this is caused by widget.macos.native-context-menus set to True.

Comment 3

[Marco Bonardo [:mak]]

Ah so this is also a blocker of bug 34572 and proton-context-menus.
Maybe some kind of invalidation problem in the menu, we may be hiding entries at the wrong time for macOS? Needs some debugging.

Comment 4

[Marco Bonardo [:mak]]

We hide the menuitems using css, we may have to actually hide them from js for native menus

Comment 5

[Marco Bonardo [:mak]]

Markus, what are the limits and requirements of native context menus regarding hiding/showing items, do they support css?

Comment 6

[Markus Stange [:mstange]]

Hi Marco, native context menus do not support CSS for hiding items. They only support the hidden and collapsed attributes for menu items. Can this code be changed to use attributes instead?

Comment 7

[Markus Stange [:mstange]]

This is a restriction that has already existed for native menubar menus, but with native context menus it now applies to context menus as well.

Comment 8

[Marco Bonardo [:mak]]

ok, then we should change the code to hide these menuitems in js rather than in css.

Comment 9

[Harry Twyford]

Attachment: Bug 1704474 - Remove pin/unpin page action context menu items. r?adw!

The bug calls for these items to be hidden with JS, but they were going to be removed anyways post-Proton. The removal of some subtests in browser/base/content/test/pageActions tests is consistent with this comment, which says that were are removing that test coverage post-Proton anyways.

Comment 10

[Harry Twyford]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3e3e10dd36f238f6c659fcebef8ad3ea9d9ed94

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba921d9eb679
Remove pin/unpin page action context menu items. r=adw,fluent-reviewers,extension-reviewers,flod,zombie

Comment 12

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/ba921d9eb679

Comment 13

[Emil Ghitta, QA [:emilghitta]]

Reproduced the issue mentioned in comment 0 using Firefox 89.0a1 (build from 2021-04-12) on macOS 10.15.

This is verified fixed using Firefox 89.0a1 (BuildId:20210414160838) on macOS 10.15 with native menus enabled.