Closed Bug 1704474 Opened 4 months ago Closed 4 months ago

Duplicated commands in context menu for icons in address bar

Categories

(Firefox :: Address Bar, defect, P2)

Unspecified
macOS
defect
Points:
2

Tracking

()

VERIFIED FIXED
89 Branch
Iteration:
89.2 - Apr 5 - Apr 18
Tracking Status
firefox89 --- verified

People

(Reporter: flod, Assigned: harry)

References

(Blocks 2 open bugs, Regressed 1 open bug)

Details

(Whiteboard: [proton-address-bar] [proton-context-menus] [mac:mr1] [priority:2a])

Attachments

(2 files)

Attached image 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)

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.

Flags: needinfo?(francesco.lodolo)
Whiteboard: [proton-address-bar]

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.

Flags: needinfo?(francesco.lodolo)

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.

Whiteboard: [proton-address-bar] → [proton-address-bar][proton-context-menus]
OS: Unspecified → macOS
Blocks: 1700679

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

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

Flags: needinfo?(mstange.moz)
Whiteboard: [proton-address-bar][proton-context-menus] → [proton-address-bar][proton-context-menus][mac:mr1]

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?

Flags: needinfo?(mstange.moz) → needinfo?(mak)

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.

Priority: -- → P2
Whiteboard: [proton-address-bar][proton-context-menus][mac:mr1] → [proton-address-bar] [proton-context-menus] [mac:mr1] [priority:2a]

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

Severity: -- → S3
Flags: needinfo?(mak)
Assignee: nobody → htwyford
Status: NEW → ASSIGNED
Iteration: --- → 89.2 - Apr 5 - Apr 18
Points: --- → 2

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.

Pushed by htwyford@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ba921d9eb679
Remove pin/unpin page action context menu items. r=adw,fluent-reviewers,extension-reviewers,flod,zombie
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch

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.

Status: RESOLVED → VERIFIED
Regressions: 1712556
You need to log in before you can comment on or make changes to this bug.