Closed Bug 1368734 Opened 2 years ago Closed 2 years ago

Add a non-mac-only exit/quit item to the photon hamburger panel

Categories

(Firefox :: Toolbars and Customization, enhancement, P1)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 55
Iteration:
55.7 - Jun 12
Tracking Status
firefox55 --- verified

People

(Reporter: Gijs, Assigned: mikedeboer)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, user-doc-complete, Whiteboard: [photon-structure])

Attachments

(2 files)

As shown in the spec ( https://mozilla.invisionapp.com/share/ENBBK0F9U#/screens/229252076 ) the hamburger panel should end with an 'exit' item at the bottom, on Linux/Windows. On Windows, this will also involve 'stealing' the ctrl-shift-q shortcut from net monitor.
Flags: qe-verify+
Priority: -- → P2
Whiteboard: [photon-structure][triage] → [photon-structure]
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Priority: P2 → P1
Version: 53 Branch → Trunk
Iteration: --- → 55.7 - Jun 12
As per https://mozilla.invisionapp.com/share/ENBBK0F9U#/screens/230516723, this button will not be there on OSX.
Comment on attachment 8872990 [details]
Bug 1368734 - Part 1 - re-assign the keyboard shortcut for the devtools Network Monitor to the Quit Application command and use 'E' instead.

https://reviewboard.mozilla.org/r/144506/#review148412

I'm good with this, and I don't think I desperately need to see this again when you add the Windows shortcut, though it would make sense to get buy-in from the devtools folks for stealing their shortcut (and in case they want to use a different one).

::: browser/components/customizableui/content/panelUI.inc.xul:657
(Diff revision 1)
> +                       tooltiptext="&quitApplicationCmdWin2.tooltip;"
> +#else
> +                       label="&quitApplicationCmd.label;"
> +#endif
> +#ifdef XP_UNIX
> +                       key="key_quitApplication"

As noted on IRC, please also add a shortcut for Windows. Then this can be collapsed into the previous set of ifdef'd lines so we end up with 1 inner ifdef.
Attachment #8872990 - Flags: review?(gijskruitbosch+bugs) → review+
Attachment #8872990 - Flags: review+ → review?(gijskruitbosch+bugs)
Comment on attachment 8872990 [details]
Bug 1368734 - Part 1 - re-assign the keyboard shortcut for the devtools Network Monitor to the Quit Application command and use 'E' instead.

https://reviewboard.mozilla.org/r/144506/#review148414

::: browser/locales/en-US/chrome/browser/browser.dtd:691
(Diff revision 2)
>  <!ENTITY goBackCmd.commandKey "[">
>  <!ENTITY goForwardCmd.commandKey "]">
>  <!ENTITY quitApplicationCmd.label       "Quit"> 
>  <!ENTITY quitApplicationCmd.accesskey   "Q">
>  <!ENTITY quitApplicationCmdMac2.label   "Quit &brandShorterName;">
>  <!-- LOCALIZATION NOTE(quitApplicationCmdUnix.key): This keyboard shortcut is used by both Linux and OSX builds. -->

I will remove this localization note.
The documentation at https://developer.mozilla.org/en-US/docs/Tools/Network_Monitor will need updating to reflect pre-56 shortcut   "Ctrl + Shift + Q ( Command + Option + Q on a Mac)" and post-56 shortcut "(Ctrl + Shift + E ( Command + Option + E on a Mac)."
Keywords: dev-doc-needed
Comment on attachment 8873092 [details]
Bug 1368734 - Part 2 - Add a Quit button to the Photon app menu on Windows and Linux.

https://reviewboard.mozilla.org/r/144544/#review148426
Attachment #8873092 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8872990 [details]
Bug 1368734 - Part 1 - re-assign the keyboard shortcut for the devtools Network Monitor to the Quit Application command and use 'E' instead.

https://reviewboard.mozilla.org/r/144506/#review148428
Attachment #8872990 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to Brian Grinstead [:bgrins] from comment #7)
> The documentation at
> https://developer.mozilla.org/en-US/docs/Tools/Network_Monitor will need
> updating to reflect pre-56 shortcut   "Ctrl + Shift + Q ( Command + Option +
> Q on a Mac)" and post-56 shortcut "(Ctrl + Shift + E ( Command + Option + E
> on a Mac)."

It's also listed on this page: https://developer.mozilla.org/en-US/docs/Tools/Keyboard_shortcuts
Comment on attachment 8872990 [details]
Bug 1368734 - Part 1 - re-assign the keyboard shortcut for the devtools Network Monitor to the Quit Application command and use 'E' instead.

https://reviewboard.mozilla.org/r/144506/#review148430

Posted to https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/P4QpnYtzJCw
Attachment #8872990 - Flags: review?(bgrinstead) → review+
Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5be871146313
Part 1 - re-assign the keyboard shortcut for the devtools Network Monitor to the Quit Application command and use 'E' instead. r=bgrins,Gijs
https://hg.mozilla.org/integration/autoland/rev/75cd29c89c8c
Part 2 - Add a Quit button to the Photon app menu on Windows and Linux. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/5be871146313
https://hg.mozilla.org/mozilla-central/rev/75cd29c89c8c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
I updated both MDN pages that were mentioned in comment 7 and comment 10.
Depends on: 1369909
Depends on: 1370442
Verified on Windows and Ubuntu.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Duplicate of this bug: 1365938
Blocks: 1387512
(In reply to Christopher Schramm from comment #19)
> https://support.mozilla.org/en-US/kb/keyboard-shortcuts-perform-firefox-
> tasks-quickly still needs an update.

Thanks for pointing this out. I added a revision on top of the 2 other revisions that are awaiting approval. Joni, can you edit/approve it?
Flags: needinfo?(jsavage)
Keywords: user-doc-needed
Thanks for updating the article. It's been published now.
Flags: needinfo?(jsavage)
You need to log in before you can comment on or make changes to this bug.