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

VERIFIED FIXED in Firefox 55

Status

()

Firefox
Toolbars and Customization
P1
normal
VERIFIED FIXED
3 months ago
20 days ago

People

(Reporter: Gijs, Assigned: mikedeboer)

Tracking

(Blocks: 2 bugs, {dev-doc-complete})

Trunk
Firefox 55
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 verified)

Details

(Whiteboard: [photon-structure])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

3 months ago
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+

Updated

3 months ago
Priority: -- → P2
Whiteboard: [photon-structure][triage] → [photon-structure]
(Assignee)

Updated

3 months ago
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Priority: P2 → P1
Version: 53 Branch → Trunk

Updated

3 months ago
Iteration: --- → 55.7 - Jun 12
(Assignee)

Comment 1

3 months ago
As per https://mozilla.invisionapp.com/share/ENBBK0F9U#/screens/230516723, this button will not be there on OSX.
Comment hidden (mozreview-request)
(Reporter)

Comment 3

3 months ago
mozreview-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/#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+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

3 months ago
Attachment #8872990 - Flags: review+ → review?(gijskruitbosch+bugs)
(Assignee)

Comment 6

3 months ago
mozreview-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/#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
(Reporter)

Comment 8

3 months ago
mozreview-review
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+
(Reporter)

Comment 9

3 months ago
mozreview-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 11

3 months ago
mozreview-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/#review148430

Posted to https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/P4QpnYtzJCw
Attachment #8872990 - Flags: review?(bgrinstead) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 14

3 months ago
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

Comment 15

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5be871146313
https://hg.mozilla.org/mozilla-central/rev/75cd29c89c8c
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
(Assignee)

Comment 16

3 months ago
I updated both MDN pages that were mentioned in comment 7 and comment 10.
Keywords: dev-doc-needed → dev-doc-complete

Updated

3 months ago
Depends on: 1369909
Depends on: 1370442
Verified on Windows and Ubuntu.
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified
Flags: qe-verify+
(Reporter)

Updated

2 months ago
Duplicate of this bug: 1365938
Blocks: 1387512
You need to log in before you can comment on or make changes to this bug.