1726736 - Implement different wording and a different pref to warn users when using a keyboard shortcut to quit

Status: VERIFIED FIXED

(Firefox :: General, enhancement, P3)

Description

[:Gijs (he/him)]

Attachment: Spec for dialog

Right now, browser.tabs.warnOnClose governs a warning that appears when closing multiple tabs.

We'll want a separate pref, browser.warnOnQuitShortcut, that defaults to true on macOS and Linux, and false on Windows.

We'll want to check this pref when the user quits the browser using the <key id="key_quitApplication">.

Right now, the quit warnings hook into this BrowserGlue.jsm code: https://searchfox.org/mozilla-central/rev/12770bd668c0a6bdaa8eb96ad9507c6febe8d23d/browser/components/BrowserGlue.jsm#2779 .

For the telemetry work in an earlier bug (bug 1712306), we added _quitSource on the BrowserGlue object; we can check if it is shortcut to see if that was the case, and if so, show an appropriate warning prompt. If we show this prompt, we should not also show the "regular" quit / close-multiple tabs prompt, even if that would otherwise have been shown; there's no point confirming it twice!

We should be able to have the single confirmEx call for quitting use either the quit shortcut text and checkbox text, or the "multiple tabs" version and checkbox text (x-ref bug 1724964), depending on our reason for showing the warning.

To include the shortcut text in the text, we'll want to grab the key element in one of the open windows for the shortcut, and use ShortcutUtils.prettifyShortcut to get text for the shortcut, that we can then insert in the message we display.

Note that this shortcut warning applies even if there is only 1 tab (pagecount == 1 in the BrowserGlue code, where it currently returns early if pagecount < 2).

Comment 1

[Neil Deakin]

There are three existing preferences related to closing tabs/quitting:

browser.warnOnQuit - hidden preference that overrides all others if false
browser.sessionstore.warnOnQuit - hidden preference that disables warning on quit when session store is active
browser.tabs.warnOnClose - disables close tab warning, corresponds to checkbox in preferences

Is the intent here (and for the other related bugs) to remove the 'browser.sessionstore.warnOnQuit' preference and use 'browser.tabs.warnOnClose' for closing or quitting without the keyboard shortcut (close button or quit from the menu), and add a new preference 'browser.warnOnQuitShortcut' for quitting with Ctrl/Cmd+Q?

Will the new 'browser.warnOnQuitShortcut' preference have UI in preferences?

Comment 2

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #1)

There are three existing preferences related to closing tabs/quitting:

browser.warnOnQuit - hidden preference that overrides all others if false
browser.sessionstore.warnOnQuit - hidden preference that disables warning on quit when session store is active
browser.tabs.warnOnClose - disables close tab warning, corresponds to checkbox in preferences

Is the intent here (and for the other related bugs) to remove the 'browser.sessionstore.warnOnQuit' preference and use 'browser.tabs.warnOnClose' for closing or quitting without the keyboard shortcut (close button or quit from the menu), and add a new preference 'browser.warnOnQuitShortcut' for quitting with Ctrl/Cmd+Q?

Yep, that's all correct. I think my assumption was that we could deal with the session store pref removal in bug 1724976.

Will the new 'browser.warnOnQuitShortcut' preference have UI in preferences?

Yes. The authoritative spec is in figma; I'll add another screenshot for the preferences portion.

Comment 3

[:Gijs (he/him)]

Attachment: Preference update for the new shortcut-related warning

The expectation is that this warning only shows up on macOS+Linux, with the relevant shortcut embedded in the message.

Comment 4

[Neil Deakin]

Attachment: Bug 1726736, add a separate preference when quitting using the shortcut key, and modify the close dialog warning to indicate that the warning only applies to keyboard quitting, r=mhowell

Depends on D124221

Comment 5

[Neil Deakin]

Attachment: Bug 1726736, add checkbox to the main preferences page for the confirm on pressing the quit keyboard shortcut setting, r=mhowell

Depends on D124222

Comment 6

[Neil Deakin]

Attachment: Bug 1726736, test for quit keyboard shortcut opening confirmation dialog, r=mhowell

Depends on D124223

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ded428a99b80
add a separate preference when quitting using the shortcut key, and modify the close dialog warning to indicate that the warning only applies to keyboard quitting, r=mhowell
https://hg.mozilla.org/integration/autoland/rev/d648841f25c8
add checkbox to the main preferences page for the confirm on pressing the quit keyboard shortcut setting, r=Gijs,fluent-reviewers,preferences-reviewers,flod
https://hg.mozilla.org/integration/autoland/rev/9f39ead09bf3
test for quit keyboard shortcut opening confirmation dialog, r=mhowell

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/ded428a99b80
https://hg.mozilla.org/mozilla-central/rev/d648841f25c8
https://hg.mozilla.org/mozilla-central/rev/9f39ead09bf3

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Is it intended to have the dialog about closing multiple windows been shown if the (Windows) user uses menu File > Quit and has automatic session restore on launch enabled?

Comment 10

[Neil Deakin]

The close multiple tabs/windows warning will appear if 'Confirm before closing multiple tabs' is checked in preferences when pressing the window close button or selecting quit/exit from the menu. The session restore setting no longer has any effect on whether this warning applies. All of this was done in bug 1724976 however. This bug is about the separate preference for the keyboard shortcut.

Comment 12

[Negritas Sergiu, Desktop QA]

Hello! I have managed to reproduce the issue with firefox 93.0a1(2021-08-20) on Windows 10. I can confirm that the issue is fixed with firefox 94.0b9 and 95.0a1(2021-10-24) on Windows 10, Ubuntu 20 and macOS 10.15.
I will update the flags and status of this issue.

Thank you!