1522039 - "Close Private Window" placed in macOS touchbar in private browsing closes window accidentally

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Environment

• https://hg.mozilla.org/mozilla-central/rev/69a2799081521866febfe2bbcbfc27b3d60133bd
• Intel Mac OS X 10.14

Steps to Reproduce

• Open multiple tabs in a private window.

What is the problem?

In the current default setting, Firefox places a large "Close Private Window" button in the center of macOS touchbar. (see attachment) if we're focusing a private browsing window.
This sometimes causes that I close the current private browsing window accidentally by my mis-touching its button. This is serious, serious problem.
If we close a private browsing browsing window, we cannnot recover its window from "Recently Closed Window" by "History" menu. The current setting can do this easily.

I seem this is too opinionated...

Expected Result

• "Close Private Window" button in the center of macOS touchbar by the default.
  • I seem that "Close this tab" button (for current focusing private browsing window) might be less trouble.
    • We can undo the closed tab even if the tab in the private browsing window.
    • But if the tab is not last one in the private window, we cannot recover it.
• Safari & Google Chrome does not change the layout of buttons in touchbar by either the window is normal or private.
  • I prefer this option. I don't have to think whether a current window is private or not.

Comment 1

[Mike de Boer [:mikedeboer]]

Interesting and very much a question for UX experts to consider. Blake, can you take a look at this usecase and perhaps forward the request?

Comment 2

[Mike de Boer [:mikedeboer]]

BTW, Tetsuharu-san, you mentioned that you'd post an attachment that show the button arrangement you talk about. It's quite easy to reproduce, but wanted to bring it to your attention nonetheless.

Comment 3

[Harry Twyford]

Hello! As Mike mentioned, I imagine UX will consider a sensible default set of buttons. A "close this tab" button is also certainly doable, should UX find that to be a better route. In the meantime, if you want to disable this behaviour, you can customize what buttons appear with the ui.touchbar.layout preference in about:config.

The button layout is defined by a comma-separated list of button names. The button that is "Search or enter address" in regular browsing and "Close" in Private Browsing Mode is called "OpenOrFocus". If you replace this string with "OpenLocation", then it will always show "Search or enter address" regardless of browsing mode.

While you're editing this string you can also rearrange buttons and add the buttons that were left out of the default set: "Forward", "Find", "Home", "Sidebar", "ReaderView" and "Fullscreen". It's a bit of a circuitous way of doing customization, but work in bug 1522012 might make it easier.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

It seems like changing the string to "Close Window" or "Close Private Browsing Window" would be a decent temporary fix, while we do more experimentation on what should actually be in the default set of buttons, and decide what the various features we expose should be…

Comment 5

[Harry Twyford]

Attachment: Bug 1522039 - Changed label on Touch Bar "Focus" button to "Close Window" to better reflect behaviour. r?bwinton,flod

Comment 6

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: actual_result.jpg

BTW, Tetsuharu-san, you mentioned that you'd post an attachment that show the button arrangement you talk about. It's quite easy to reproduce, but wanted to bring it to your attention nonetheless.

Ah. I'm sorry. I forget to attach it.
This photo is the current status of my laptop's keyboard which I filed a problem as this bug.
I sometime miss-touch this "Close the current private window" button on typing the keyboard :(

Comment 7

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Harry Twyford [:harry] from comment #3)

In the meantime, if you want to disable this behaviour, you can customize what buttons appear with the ui.touchbar.layout preference in about:config.

Thank you for your guide.

But, I'm thinking as an issue that setting 'Close Window' button as a large center button in private window by default.
It's not the problem about UI labels...

Comment 8

[Blake Winton (:bwinton) (:☕️)]

(In reply to Tetsuharu OHZEKI [:tetsuharu] [UTC+9] from comment #7)

I'm thinking as an issue that setting 'Close Window' button as a large center button in private window by default.
It's not the problem about UI labels...

Yes, changing the label isn't the final solution for this, just a temporary hold-over until we figure out what the set of buttons should be. 🙂

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49392773ff29
Changed label on Touch Bar "Focus" button to "Close Window" to better reflect behaviour. r=bwinton,flod

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/49392773ff29

Comment 11

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Blake Winton (:bwinton) (:☕️) from comment #8)

(In reply to Tetsuharu OHZEKI [:tetsuharu] [UTC+9] from comment #7)

I'm thinking as an issue that setting 'Close Window' button as a large center button in private window by default.
It's not the problem about UI labels...

Yes, changing the label isn't the final solution for this, just a temporary hold-over until we figure out what the set of buttons should be. 🙂

But this bug has been fixed by landing https://hg.mozilla.org/mozilla-central/rev/49392773ff29.

Should I open a new bug to track changing the default button sets in touchbar?

Comment 12

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Tetsuharu OHZEKI [:tetsuharu] [UTC+9] from comment #11)

But this bug has been fixed by landing https://hg.mozilla.org/mozilla-central/rev/49392773ff29.

"this bug has been fixed" -> "this bug has been closed"

Comment 13

[Mike de Boer [:mikedeboer]]

No, let's keep this one open.

Comment 14

[Blake Winton (:bwinton) (:☕️)]

Relatedly, we have a very small amount of telemetry on which buttons get used the most, which should be helpful in determining the default set of bugs…

Comment 15

[Stephen Horlander [:shorlander]]

(In reply to Blake Winton (:bwinton) (:☕️) from comment #8)

(In reply to Tetsuharu OHZEKI [:tetsuharu] [UTC+9] from comment #7)

I'm thinking as an issue that setting 'Close Window' button as a large center button in private window by default.
It's not the problem about UI labels...

Yes, changing the label isn't the final solution for this, just a temporary hold-over until we figure out what the set of buttons should be. 🙂

I don't think we should do anything specific to PBM in the Touch Bar by default.

Private Browsing doesn't change the browsing context and we are replacing a functionality that exists in both modes (Search or Enter Address) with something that at minimum is frustrating if you hit it on accident and at worst can be responsible for data loss.

Comment 16

[Harry Twyford]

(In reply to Stephen Horlander [:shorlander] from comment #15)

something that at minimum is frustrating if you hit it on accident and at worst can be responsible for data loss.

Good point. Would there be any objection to making the default button the persistent (across regular and PBM) "Search or enter address" button and leaving the behaviour-changing button as an option for those who want to mess around with about:config?

One other option: we could also add a PBM-specific Clear Window button that's the size of a regular Touch Bar button (not the extra-wide middle button). Icon would be the PBM mask, it would still appear PBM-purple, and it would still have the Close Window/Tab behaviour. This way it would appear as an extra button, rather than replacing "Search or enter address", and it'd be a smaller touch target, reducing the chance of an accidental press.

Related: If I update the default ui.touchbar.layout pref in modules/libpref/init/all.js, does that update the pref for all users who haven't yet modified it?

Comment 17

[Harry Twyford]

Attachment: sample Close Window behaviour.png

Admittedly, this approach creates the issue of it not being clear what the button does. This could be solved with a more descriptive icon? Or we just make "Search or enter address" the persistent default with no special Close Window button!

Comment 18

[Stephen Horlander [:shorlander]]

(In reply to Harry Twyford [:harry] from comment #17)

Created attachment 9039230 [details]
sample Close Window behaviour.png

Admittedly, this approach creates the issue of it not being clear what the button does. This could be solved with a more descriptive icon? Or we just make "Search or enter address" the persistent default with no special Close Window button!

Yes. I think it's best not to insert potentially destructive mode errors by default :)

We could potentially mitigate this by making it a more obvious purple trash can (see Focus) but that still doesn't address accidental touches. Which would maybe be less of a concern on a physical button, but more likely on a touch responsive surface.

Leaving it in as a customization option is probably fine. At least people know what they are getting into if they add it.

Comment 19

[Harry Twyford]

Sounds good. Could you or someone in UX please produce a white trash can icon in .pdf like those in widget/cocoa/touchbar? I went to the icon site but it looks like the light-coloured export for the trash can icon isn't working: https://design.firefox.com/icons/viewer/#trash

I'll push a patch that makes the Search or enter address button the persistent default and changes the now-optional close window button icon to the trash can.

Comment 20

[Blake Winton (:bwinton) (:☕️)]

Attachment: delete.pdf

How's this?

Comment 21

[Blake Winton (:bwinton) (:☕️)]

(And while I'm here, what do you think about making the default set more like this?)

Comment 22

[Harry Twyford]

Attachment: Bug 1522039 - Changes default Touch Bar button set and replaces icon on Close Window button. r?bwinton

Comment 23

[Ron Akanowicz, UX (needinfo me for questions)]

Hi folks, I'm a new UX resource under Markus. I, along with two colleagues, read over this and would like to recommend we use the same Touch Bar as is used for regular browsing.

• Other browsers, like Safari and Chrome, use a consistent bar between modes.
• We don't recommend the Touch Bar contain "destructive" actions (close, delete, remove, etc.)
• We don't recommend changing the function or layout of buttons between the regular and private modes.

I hope this helps.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c5bbf3bb4d8
Changes default Touch Bar button set and replaces icon on Close Window button. r=bwinton,mikedeboer

Comment 25

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/7c5bbf3bb4d8

Comment 26

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/7c5bbf3bb4d8

Comment 27

[Harry Twyford]

Comment on attachment 9039550 [details]
Bug 1522039 - Changes default Touch Bar button set and replaces icon on Close Window button. r?bwinton

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1522039
• User impact if declined: A "Close Window" button remains in the Touch Bar in Private Browsing Mode. This may cause accidental data loss. Furthermore, If we don't take this patch the Touch Bar feature won't be enabled for users, as per bug 1529634.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Removes, rather than adds, a button to the MacBook Pro Touch Bar.
• String changes made/needed:

Comment 28

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello,

Confirming this issue as verified fixed on the latest FF Nightly build 67.0a1(20190226093755) on macOS 10.14.

Comment 29

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Thank you for fixing this!

Comment 30

[Liz Henry (:lizzard)]

Setting this back to affected for 66. (Usually it's better to open a new bug for follow up fixes, though.)

Comment 31

[Liz Henry (:lizzard)]

Comment on attachment 9039550 [details]
Bug 1522039 - Changes default Touch Bar button set and replaces icon on Close Window button. r?bwinton

Fix verified in nightly; let's uplift for beta 12.

Comment 32

[Razvan Maries]

https://hg.mozilla.org/releases/mozilla-beta/rev/fc92378333dc

Comment 33

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Confirming this issue as verified fixed on the latest FF Beta build 66.0b12(20190228180200) on macOS 10.14.3