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

Status: VERIFIED FIXED

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

Description

[:Gijs (he/him)]

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.

Comment 1

[Mike de Boer [:mikedeboer]]

As per https://mozilla.invisionapp.com/share/ENBBK0F9U#/screens/230516723, this button will not be there on OSX.

Comment 2

[Mike de Boer [:mikedeboer]]

Attachment: Bug 1368734 - Part 1 - re-assign the keyboard shortcut for the devtools Network Monitor to the Quit Application command and use 'E' instead.

Review commit: https://reviewboard.mozilla.org/r/144506/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/144506/

Comment 3

[:Gijs (he/him)]

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.

Comment 4

[Mike de Boer [:mikedeboer]]

Attachment: Bug 1368734 - Part 2 - Add a Quit button to the Photon app menu on Windows and Linux.

Review commit: https://reviewboard.mozilla.org/r/144544/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/144544/

Comment 5

[Mike de Boer [:mikedeboer]]

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.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144506/diff/1-2/

Comment 6

[Mike de Boer [:mikedeboer]]

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.

Comment 7

[Brian Grinstead [:bgrins]]

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)."

Comment 8

[:Gijs (he/him)]

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

Comment 9

[:Gijs (he/him)]

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

Comment 10

[Brian Grinstead [:bgrins]]

(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 11

[Brian Grinstead [:bgrins]]

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

Comment 12

[Mike de Boer [:mikedeboer]]

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.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144506/diff/2-3/

Comment 13

[Mike de Boer [:mikedeboer]]

Comment on attachment 8873092 [details]
Bug 1368734 - Part 2 - Add a Quit button to the Photon app menu on Windows and Linux.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144544/diff/1-2/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5be871146313
https://hg.mozilla.org/mozilla-central/rev/75cd29c89c8c

Comment 16

[Mike de Boer [:mikedeboer]]

I updated both MDN pages that were mentioned in comment 7 and comment 10.

Comment 17

[u554753]

Verified on Windows and Ubuntu.

Comment 19

[Christopher Schramm]

https://support.mozilla.org/en-US/kb/keyboard-shortcuts-perform-firefox-tasks-quickly still needs an update.

Comment 20

[:Gijs (he/him)]

(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?

Comment 21

[Joni Chan]

Thanks for updating the article. It's been published now.