1692669 - [Proton] Replace icon group navigation on macOS page context menu with "normal" menu items

Status: VERIFIED FIXED

(Firefox :: Menus, enhancement, P1)

Description

[:Gijs (he/him)]

To help with the appearance of the menu on macOS, we're replacing the group of icons with 4 items and slightly changing where the separator is. The menu's top should be:

Back
Forward
Reload
---
Bookmark Page

immediately followed (no separator) by other save/share/send page items in their current order.

We'll make this change only on macOS; the Windows/Linux context menu should stay how it is in this respect.

We're making a number of other changes in other bugs that will mean the overall length of the context menu on macOS doesn't change substantially.

Comment 1

[:Gijs (he/him)]

Jared, are you OK to still work on this?

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

I'm going to upload my patch that I have in-progress. There are a couple test failures on mac that need updating due to the new menu organization.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1692669 - Replace icon group navigation on macOS page context menu with 'normal' menuitems.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

I'm unassigning myself since I haven't had time to track down the context menu test failures. There are failures in /accessible/tests/browser/mac/browser_app.js and there may still be failures in /browser/base/content/test/contextMenu/browser_contextmenu.js

Comment 5

[Markus Stange [:mstange]]

Try push with patch: https://treeherder.mozilla.org/jobs?repo=try&revision=4f9b27c62aa00c57b9e6dbf2408796e9fba4963a&selectedTaskRun=NjEyqbk_TGasvll2qNl6cg.0

Test failures in:

• /accessible/tests/browser/mac/browser_app.js
• /browser/base/content/test/contextMenu/browser_contextmenu.js
• /browser/components/extensions/test/browser/browser_ext_browserAction_contextMenu.js
• /browser/components/extensions/test/browser/browser_ext_menus_replace_menu.js
• /browser/components/extensions/test/browser/browser_ext_menus_replace_menu_context.js
• /browser/components/extensions/test/browser/browser_ext_sidebarAction_contextMenu.js

In addition to the patch I also removed the include of shared/contextmenu.inc.css from the macOS browser.css, so that the menu items don't have icons.

The other missing piece in the patch is the handling of the separator line under the "Bookmark Page" item; as comment 0 says, there should be no separator between "Bookmark Page" and "Save Page As..." but with the current patch there still is a separator.

Comment 6

[Markus Stange [:mstange]]

I'm not currently working on this bug. I may get to it once I'm done with other native context menu work, if nobody beats me to it.

Comment 7

[Markus Stange [:mstange]]

Oh, the current patch also causes Back/Forward to show up when right-clicking a link. Before this patch, context menus for links did not include the icon buttons.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ed28d4ec972
Replace icon group navigation on macOS page context menu with 'normal' menuitems. r=flod,mconley

Comment 9

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/2ed28d4ec972

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 11

[Emil Ghitta, QA [:emilghitta]]

This is verified fixed using Firefox 89.0a1 (BuildId:20210323094659) on macOS 10.14. The icon group navigation was successfully replaced with the "normal" context menu items.