1591040 - Cannot active contextMenu on web extension icon when firefox is in fullscreen mode

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect, P3)

Description

[abc]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:70.0) Gecko/20100101 Firefox/70.0

Steps to reproduce:

1. open firefox on macOS system
2. click top-left green button to request system fullscreen of firefox
3. right click on web extension icon

Actual results:

Web extension contextMenu cannot show, but show firefox contextMenu(hide toolbar and exit fullscreen).

In this case, I can not use contextMenu of web extension when firefox in macOS system fullscreen.

Expected results:

Should show web extension contextMenu

Comment 1

[Timea Cernea [:tbabos][inactive]]

Hey abc,

Reproduced this issue on the latest Firefox72, FX71 and FX70 on MacOS and Windows 10 as well.
The fullscreen toolbar context menu is shown if I right-click any web extension icon.
Attached recording of the issue.
Thanks for the report!

Comment 2

[Timea Cernea [:tbabos][inactive]]

Attachment: Recording of the issue -Win10

Comment 3

[:Gijs (he/him)]

None of the builtin items show a context menu either, so I think this sort of makes sense? Up to the webextension team if they think webextensions need special treatment here (and why).

Comment 4

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

The priority flag is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 7

[sheelgautam995]

Some other context menus are also not shown in addition to the extension menu.

Comment 8

[sheelgautam995]

This may not be a big issue in Windows but on MacOS its a bit more significant because the fullscreen mode works differently in MacOS.

Comment 9

[Shane Hughes [:aminomancer]]

I don't find the fullscreen toolbar context menu particularly useful. It's not useless, but is it really useful enough that it should not only get its own context menu, but actually disable the much larger toolbar context menu? It's not just that it deprives the user of those functions in fullscreen.

It may also create some kind of cognitive dissonance because the user gets used to seeing the toolbar context menu when right-clicking toolbar buttons. Just speculation, but discovering that you can't access that menu in fullscreen might discourage the user from entering fullscreen mode at all. I feel like I would use fullscreen more often were it not for this issue.

There are only two menuitems in the menu, and I doubt that either is likely to be used frequently. Maybe there's user research on this but I assume that turning off the toolbox auto-hide feature is not something the average user is likely to change frequently, and exiting fullscreen is probably more commonly done with the keyboard shortcut, the toolbarbutton, or the app menu button.

Both menuitems can be added to the ordinary toolbar context menu, and disabled programmatically in the same conditions under which the context itself is switched from the toolbar context menu to the fullscreen context menu. The toolbar can be customized in fullscreen mode anyway. Of course, the user can enter customize mode and then enable fullscreen with the keyboard shortcut, without any problems afaik.

It feels inconsistent too, because there are .customize-context-* menuitems in both the toolbar context menu and the customization palette context menu. While you're in fullscreen, you can't use the toolbar context menu to "pin to overflow panel" or "unpin from overflow panel" or "remove from toolbar," but you can use the palette context menu to "add to overflow panel" or "add to toolbar." Which are basically parallel functions. I think the full suite of commands should be available from context menus in fullscreen — at least in customize mode, but preferrably in any case.

I would like to make a patch removing the fullscreen context menu #autohide-context and moving its menuitems to the toolbar context menu #toolbar-context-menu if someone will submit it.

Comment 10

[:Gijs (he/him)]

Katie, could you take a look at this / bug 1393749?

In full screen, the toolbar context menu contains just two entries: "exit fullscreen" and the checkable "Hide toolbars" item.

I agree with comment #9 to the extent that I think we should not override the normal context menu in full screen - I think we should show those two fullscreen items at either the end or start of the usual context menu, when it appears. Does that sound OK?

Comment 11

[:Gijs (he/him)]

UX had a look at this and put up a plan in https://www.figma.com/file/cunx7mGzJVW8TdtiKaBhkq/MR1.1-%2F-MR2---Desktop?node-id=2288%3A19323 (unfortunately not public, but basically agrees with the previous suggestion of appending these fullscreen items to the "normal" toolbar menu in fullscreen).

(In reply to aminomancer from comment #9)

I would like to make a patch removing the fullscreen context menu #autohide-context and moving its menuitems to the toolbar context menu #toolbar-context-menu if someone will submit it.

This sounds great to me if you're still willing to do this. We might also want to add the same items to the tabstrip context menu?

If we could update the label for the check-able "auto-hide" item to "Hide toolbar and tabstrip" at the same time, that would be great (but if not, I can also add that to the patch.

Comment 12

[Shane Hughes [:aminomancer]]

(In reply to :Gijs (he/him) from comment #11)

If we could update the label for the check-able "auto-hide" item to "Hide toolbar and tabstrip" at the same time, that would be great (but if not, I can also add that to the patch.

I think the label in English is "Hide Toolbars" currently, which seems consistent. Idk if there's any user-facing confirmation of this, or any textual reference to the tabstrip at all. But in the source code the tabstrip is just the specific area name of a customizable toolbar area with a DOM id TabsToolbar. And the user has probably noticed they can drag customizable widgets into the same horizontal row in which the tabstrip exists, and the primary affordance leading into that interface is the context menu item that says "Customize Toolbar..."

So I do think it makes sense to say you're hiding the toolbars. I don't think there is any reference to terms like the navigator or nav toolbox either. But I'm sure there are still parts of the browser I haven't stumbled onto. There isn't any way to enable or disable it so unlike the other toolbars there isn't any button or checkbox that refers to it by name. I guess strictly speaking "Hide Toolbar and Tabstrip" is harder to misinterpret so maybe that's the right way to go, even if it's not as elegant as "Hide Toolbars"

I agree the items should be available in the tabstrip context menu. Or the items should be in every context menu that is currently replaced with the autohide context menu. I believe that includes every toolbar that has the attribute fullscreentoolbar=true and I think that includes just the tabstrip and the main customizable toolbar. Bookmarks toolbar is hidden in fullscreen so we can skip that. There might be something else I'm missing but I'll work on just the toolbar and tabstrip.

Comment 13

[:Gijs (he/him)]

(In reply to aminomancer from comment #12)

There might be something else I'm missing but I'll work on just the toolbar and tabstrip.

Nope, this all makes sense to me - thanks for working on this!

Comment 14

[Shane Hughes [:aminomancer]]

I also think if we're putting it in the toolbar context menu, the auto-hide item belongs in the View > Toolbars menu too. All the items in the "group" in which I placed the auto-hide item (the items for hiding specific toolbars and the customize toolbar item) have a counterpart in that View > Toolbars menu. And an enter/exit fullscreen checkbox item is already present in the View menu. So it kinda makes sense to add auto-hide there too, but with a slightly different organization. So, while in fullscreen, that would yield a hierarchy of View > Toolbars > Hide Toolbars in Full Screen; and a separate hierarchy of View > Full Screen.

Considering this is going to be visible in these toolbar contexts that don't exclusively exist in fullscreen, and it won't be immediately obvious to the user that they only pertain to fullscreen mode until they've tried opening the context menu in both fullscreen and non-fullscreen modes, it occurred to me that maybe the label should be "Hide Toolbars in Fullscreen" or something along those lines.

Even though it's obvious to us, as I'm testing this, it looks like a possible source of confusion because it's not visually apparent why the auto-hide menuitem appeared. The "exit full screen" item is obvious but for the auto-hide item, you'd first have to observe that it doesn't appear in normal mode and make note of that. So upon seeing the item, the user may not immediately assume that it only shows in fullscreen mode. So they might think "Hide Toolbars" is supposed to have the effect of hiding the toolbars all the time.

I went ahead and changed the string to "Hide Toolbars in Full Screen" for that reason. The other string full-screen-exit reads "Exit Full Screen Mode" but it seems like the odd duck. In the View menu, there's an item whose label is just "Full Screen", not "Full Screen Mode". And that is repeated everywhere else I could find. So for the same reason, I'm changing the string full-screen-exit from "Exit Full Screen Mode" to "Exit Full Screen", since these items will be right next to each other. Not only is it shorter, but the phrase "Full Screen Mode" doesn't appear anywhere else in browser.ftl. Although the phrase "full screen mode" is used often in documentation, in the actual UI it always seems to be referred to as just "Full Screen". So I'm sticking with that, but I'm sure someone will correct me if that's wrong.

P.S. I guess one could argue that you don't need to put the items in the #tabContextMenu (the one that opens when you right click an actual tab) because that menu is and was still accessible in fullscreen, and because right clicking the empty space or non-tabbrowser widgets in the tabstrip area gets you the toolbar context menu. But I put them in at the end anyway, and others can decide whether that's worth keeping.

I still can't get mach bootstrap to work so I can't commit it myself. Hopefully you or someone can download it and submit for review. I'm cc'd so I guess just comment if I borked something. 🙏

Comment 15

[Shane Hughes [:aminomancer]]

(In reply to :Gijs (he/him) from comment #13)

(In reply to aminomancer from comment #12)

There might be something else I'm missing but I'll work on just the toolbar and tabstrip.

Nope, this all makes sense to me - thanks for working on this!

Any news on this? Did my patch work?

Comment 16

[:Gijs (he/him)]

(In reply to aminomancer from comment #15)

(In reply to :Gijs (he/him) from comment #13)

(In reply to aminomancer from comment #12)

There might be something else I'm missing but I'll work on just the toolbar and tabstrip.

Nope, this all makes sense to me - thanks for working on this!

Any news on this? Did my patch work?

Hi, sorry, I've been off ill for a while and trying to play catch-up since. Because the patch isn't on phab it doesn't appear in my queue and so I'm relying on an unread bugmail in my bugmail folder to keep track of this, but I get like 100 of those every day, so it's gotten a bit buried. I'll try and take a look now.

Comment 17

[:Gijs (he/him)]

When creating the patch based on the github repo's contents, do you want crediting as just "aminomancer" with your bugzilla email, or are there other details you'd like me to use?

Comment 18

[:Gijs (he/him)]

(In reply to aminomancer from comment #14)

I also think if we're putting it in the toolbar context menu, the auto-hide item belongs in the View > Toolbars menu too. All the items in the "group" in which I placed the auto-hide item (the items for hiding specific toolbars and the customize toolbar item) have a counterpart in that View > Toolbars menu.

Well, the per-button context menus or per-tab context menus don't, and the context menus are just that - contextual. So it sort of makes sense to me that we bring up these items when in fullscreen, but not outside of fullscreen. I can see your argument, but I think that either way it's better solved in a separate bug - this bug is about the fact that the other ("normal") items in the context menu become inaccessible in full screen, and the full screen context menu's items are kind of orthogonal to that.

And an enter/exit fullscreen checkbox item is already present in the View menu. So it kinda makes sense to add auto-hide there too, but with a slightly different organization. So, while in fullscreen, that would yield a hierarchy of View > Toolbars > Hide Toolbars in Full Screen; and a separate hierarchy of View > Full Screen.

Considering this is going to be visible in these toolbar contexts that don't exclusively exist in fullscreen, and it won't be immediately obvious to the user that they only pertain to fullscreen mode until they've tried opening the context menu in both fullscreen and non-fullscreen modes, it occurred to me that maybe the label should be "Hide Toolbars in Fullscreen" or something along those lines.

Right, to me this is another reason to do it in a separate patch. It might make sense to use a different label there, and we might also want the item to appear dynamically but directly in the top "View" menu. Those are all UX questions for which I'd need to go back to UX and ask them to spend time thinking about this, and we don't need to do that to address the root problem people are running into in this bug.

The other string full-screen-exit reads "Exit Full Screen Mode" but it seems like the odd duck. In the View menu, there's an item whose label is just "Full Screen", not "Full Screen Mode". And that is repeated everywhere else I could find. So for the same reason, I'm changing the string full-screen-exit from "Exit Full Screen Mode" to "Exit Full Screen", since these items will be right next to each other. Not only is it shorter, but the phrase "Full Screen Mode" doesn't appear anywhere else in browser.ftl. Although the phrase "full screen mode" is used often in documentation, in the actual UI it always seems to be referred to as just "Full Screen". So I'm sticking with that, but I'm sure someone will correct me if that's wrong.

This makes sense to me, but without the change in the other item's label it doesn't look that odd. Changing the label will require changing the string ID and retranslating it in all our localizations, which is also tedious, and it needs someone from UX to stamp the change. When they worked on this bug they changed the label for the toolbar item but not this one, so I'm not sure how they feel about it - again, a separate bug seems preferable.

P.S. I guess one could argue that you don't need to put the items in the #tabContextMenu (the one that opens when you right click an actual tab) because that menu is and was still accessible in fullscreen, and because right clicking the empty space or non-tabbrowser widgets in the tabstrip area gets you the toolbar context menu. But I put them in at the end anyway, and others can decide whether that's worth keeping.

I still can't get mach bootstrap to work so I can't commit it myself. Hopefully you or someone can download it and submit for review. I'm cc'd so I guess just comment if I borked something. 🙏

No worries, I'll put up the patch as a WIP while waiting for the answer about what to use for your username / credit. :-)

Comment 19

[:Gijs (he/him)]

Attachment: Bug 1591040 - show both fullscreen and normal toolbar/tabstrip context menus in full screen, r?jaws

Comment 20

[Shane Hughes [:aminomancer]]

All good, sorry it took me so long to catch this. Yeah aminomancer is fine, or you can just omit me. And yeah I see your point. I tend to overdo things and don't use the menubar much, but you're right, it's probably too specific to be a semi-permanent fixture of the otherwise concise menu. Another bug I posted about recently was an issue with the label of the #menu_close item in the File menu. (which has been fixed now) It only shows if you open the menu with keyboard, i.e., Alt+F. I guess that is one way of making it more contextual.

It seemed odd to have a menuitem that seems so specific to a single tab, but then I noticed the "Save Page As" item is in the same menu, and that one's always visible, even on about:blank or something. So I figured it wasn't much of a stretch to add fullscreen related menuitems since they at least pertain to the whole window. But I can see how that philosophy could wind up cluttering those menus really fast, and that change should belong in an enhancement request.

Comment 21

[Shane Hughes [:aminomancer]]

I have an account on phabricator too. Or maybe it's the same as my bugzilla account, can't remember.

Comment 22

[:Gijs (he/him)]

(In reply to aminomancer from comment #20)

All good, sorry it took me so long to catch this.

Hey, if anything, the apologies are on my side - another 2 weeks appear to have passed. :-(

Let me try to get this into shape now...

Comment 23

[Shane Hughes [:aminomancer]]

Is there anything I can do to fix the remaining issues with the patch?

Comment 24

[:Gijs (he/him)]

(In reply to aminomancer from comment #23)

Is there anything I can do to fix the remaining issues with the patch?

I think we're waiting on content design (internal link, sorry) for the use of "tabstrip" given flod's concern on phabricator that it'd be too long in other locales. Meridel, do you have an update for us? (Sorry for not reminding you sooner!)

Other than that, I think really we should figure out why moz-phab doesn't work for you, then you can update patches yourself... but probably not in this bug.

Comment 25

[Meridel [:meridel]]

Hi Flod, would "Hide toolbar and tabs" work? Still too long? The umbrella term for what gets hidden is "browser chrome" but I don't think that nomenclature is widely understood, and could be confused with Google Chrome.

And, apologies for the delay from me. This fell off my radar in the end-of-year hustle. Thank you for reminding me, Gijs!

Comment 26

[Francesco Lodolo [:flod]]

(In reply to Meridel [:meridel] from comment #25)

Hi Flod, would "Hide toolbar and tabs" work? Still too long? The umbrella term for what gets hidden is "browser chrome" but I don't think that nomenclature is widely understood, and could be confused with Google Chrome.

I agree on avoiding "chrome", it's just confusing at this point in history.

"toolbar and tabs" has the same risk in terms of lenght, e.g. (trying to guess 3/4):

• Italian: Nascondi barra strumenti e schede
• French: Masquer la barre d’outils et onglets
• German: Symbolleiste und Tabs verstecken
• Spanish: Ocultar barra de herramientas y pestañas

If that's acceptable from a UX/UA perspective, it works for me. Maybe we can add a comment suggesting to just translate as "Hide tabs" if the translations gets too long?

Comment 27

[Meridel [:meridel]]

There's no perfect answer here, but reflecting back, the original goal of this bug was to make menu items consistently accessible, not to try to improve the current labels. I don't like the idea of introducing different versions in different locales unless we are gaining a great deal and I don't think we are here. Below are the current options and their pros and cons. I think we should just keep the current string, Option 1. Apologies for the delay on this, and thank you for helping me to consider alternate options. Now we at least have documentation on this decision should it come up again.

Option 1: Hide toolbars RECOMMENDED
PRO: This is the current string so we some users will be used to it (by keeping it the same we might avoid some confusion), and it's short enough to not cause issues for localization. Since this is the current string I assume we save some localization work, too.
CON: It doesn't accurately capture the things that get hidden (the tab strip and the toolbar).

Option 2: Hide toolbar and tab strip
PRO: Accurately captures the things that get hidden.
CON: Causes localization issues in terms of length. Could be veering into too technically accurate territory and thus causing increased cognitive load.

Option 3: Hide tabs
PRO: Short (no localization issues) and plain language (no technical jargon).
CON: You access this option from a toolbar so emphasizing the tab element of the function (rather than the toolbar) might not align with expectations ("I am clicking on the toolbar, the option should be about the toolbar, not the thing above it"). It doesn't accurately capture the things that get hidden (the tab strip and the toolbar).

Flod, okay with this?

Comment 28

[Francesco Lodolo [:flod]]

(In reply to Meridel [:meridel] from comment #27)

Flod, okay with this?

Option 1 sounds good to me, thanks for providing a clear summary.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e23259733a51
show both fullscreen and normal toolbar/tabstrip context menus in full screen, r=jaws,Gijs

Comment 30

[Norisz Fay [:noriszfay]]

Backed out for causing bc failures on browser_fullscreen_context_menu.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/9ba1714ac61f5dbe9fefc7714075f0bd0b319d11

Push with failures: failure push

Failure log(s): failure log

Failure line(s): TEST-UNEXPECTED-FAIL | browser/base/content/test/fullscreen/browser_fullscreen_context_menu.js | Context menu has the expected number of items - Got 6, expected 7

Comment 31

[Shane Hughes [:aminomancer]]

Fixed the test failure for macOS

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b9d5bf8fba7
show both fullscreen and normal toolbar/tabstrip context menus in full screen, r=jaws,Gijs

Comment 33

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/5b9d5bf8fba7

Comment 34

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

I managed to reproduce this bug using the STR in the description on Firefox 70.0(20191016161957) on macOS Big Sur 11. This issue seems to be fixed on Firefox 98.0b3(20220210185745) and the latest Nightly 99.0a1(20220210213101) on the same OS.