456382 - mousing on last tab shouldn't close window (middle click, context menu)

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, enhancement)

Description

[Tuukka Tolvanen (sp3000)]

2008-09-22-10Z Firefox/3.1b1pre
 1. Ensure that prefs -> Tabs -> Always show tab bar is checked
 2. Open new browser window (e.g. ctrl-n)
 3. Middle click on the tab head
result:
 window closes
expected:
 window doesn't close

It's a bit odd to close the window container on a click targeted at the tab container. This is more or less the same issue that motivates bug 455990.

Comment 1

[Jo Hermans]

This is a fall-out from bug 392870. See bug 455852 and bug 455990

Comment 2

[Tuukka Tolvanen (sp3000)]

Attachment: patch1

Differentiates cases where tab close comes from a click at the tab widget (middle click on tab; click on tab close button though bug 455990 covers that from another angle), to take on the last tab the same path as closeWindowWithLastTab==false, i.e. close tab then open tab.

Comment 3

[Tuukka Tolvanen (sp3000)]

Attachment: patch2

oops, I missed a caller and inadvertently fixed a bug where swapBrowsersAndCloseOther would let _beginRemoveTab close the source window of a tab migration in the middle of that operation, leaving the new window with an uncloseable (removeProgressListener fail) tab. Let's make that more explicit.

Comment 4

[Dão Gottwald [:dao]]

This interferes with the patch in bug 456002.

Comment 5

[Tuukka Tolvanen (sp3000)]

Attachment: patch3 on bug 456002 attachment 339788

updated for bug 456002 attachment 339788 [details] [diff] [review]

Comment 6

[Martijn Wargers (dead)]

Attachment: browserchrome test

This is a browserchrome test that might be useful. The patch attached to this bug passes that test.
However, with the patch, I get to see two tabs, for a brief moment. Probably because my machine is relatively slow and I'm using a debug build, but still, it might be something that is noticeable also in normal builds.

Comment 7

[Dão Gottwald [:dao]]

It needs to be decided if we do this and bug 455990 or bug 456425.

Comment 8

[morboon]

What I think is the best compromise is when clicking close button on the last tab, it is replaced with an about:blank tab w/o a close button (nor close tab context menu, nor close tab keyboard shortcut, etc.). There are several benefits to this: 

1) there's no room for old-time Firefox users to accidentally close the window
2) we still retain the same Firefox behavior
3) the UI seems responsive, as the closing tab will be replaced with a blank one
4) users can't invoke close tab mechanism on a blank tab repeatedly and misunderstand that UI is unresponsive, since we already strip out all tab closing mechanism.
5) The disappearing close tab button on a blank tab will serve as a really good visual cue to let the user know that the last blank tab can't be closed. If they want to quit using Firefox, just close it normally.

Comment 9

[Tuukka Tolvanen (sp3000)]

I'd prefer to keep this bug focused on the specific mouse interaction regression from fx2-3.0 in the always-show-tabbar case, and not spill into suggestions involving changes from fx2-3.0 default keyboard behavior.

Comment 10

[Dão Gottwald [:dao]]

You'll probably have a better chance to get this ui-reviewed if you make middle clicking a no-op for the last tab.

Comment 11

[Grey Hodge (jX)]

This is a bad behavior, this is the magical "make computer explode" button that
your grandmother is always afraid of finding. This is counterintuitive for
users of all experience levels. It may make sense in theory, but theory needs
to give way to practical application. You're closing an app when a user is ONLY
closing a document. I am aware of NO application with similar behavior where
closing a document closes the entire application.

Example, in MS Word when you close a document, the app stays open. You can
create a new document, open an existing one, or stare at the pretty UI, but the
app remains until the user explicitly closes it. Opera does not close the app
when closing the last tab. Neither Safari nor IE have the ability to close the
last tab.

Basically, in Windows at least, there are well defined methods to closing an
app. The "red X" and File>Close/Exit/Quit. Double clicking the app icon in the
upper left is a holdover from Windows 3.1, and most users won't discover this
since that icon really has no other function used by most people. However, NO
application I am aware of has a section of the UI that equates to a hidden Quit
function. This behavior is counterintuitive, breaks user expectations, and can
be disruptive to the workflow of people who use it as a function to merely
clear the sole tab. The tab bar is off by default when there's one tab, so most
users will never use this feature anyway. Those who WOULD see this feature
would most certainly expect not to find it.

I'm not complaining solely because I discovered it and it's incredibly
annoying, I'm arguing against it because I truly do feel this is NOT in the
best interest of users. If this MUST stay, I feel VERY strongly that it needs
to be off by default. This is just not a beneficial behavior.

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #7)
> It needs to be decided if we do this and bug 455990 or bug 456425.

We definitely want to do bug 455990 (hide close button on last tab).
I do not think we want to do bug 456425 (hybrid clear/close behaviour).

Middle click is an odd question. It's an advanced accelerator specifically for tab functionality (unlike cmd/ctrl-w which always maps to "close"). At the very least it needs to be a no-op on the last tab, as I agree with jX in that a middle click on the last tab probably resulting in the window closing wouldn't match user expectations.

I think I could get behind it acting as "clear and keep open" as well, since it's already a pretty power user sort of thing.

Comment 13

[Tuukka Tolvanen (sp3000)]

Attachment: patchB1, the no-op edition

My spider-sense tells me that precious-window mode (closeWindowWithLastTab==false) wants a close button (via userChroming after bug 455990 if nothing else) and middle-click on the last tab that matches its flavor of document-close. This breaks that, but the upside is that middle-click behavior matches close-button presence after bug 455990 in both modes.

Comment 14

[Tuukka Tolvanen (sp3000)]

Attachment: patchA4, cover context-close too

Oops, I missed the close item in the tab's context menu. Maybe it should be disabled on the last tab as the close button is hidden, but as long as it's available it should not close the window or fail to do anything.

I think this patch is the right approach because it just makes the available mousey affordances do something that's reasonable enough in both modes. Disabling in some cases (or all, modulo potential wrath of the precious-window contingent) some of those affordances (or all, for consistency) doesn't conflict with this and probably shouldn't live in the scope of this plain ol' uncontroversial regression fix.

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 340733 [details] [diff] [review]
patchA4, cover context-close too

Either disable or replace with "Close Window" - the latter seems a little nicer to me.

Comment 16

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/e30f1765a42c

Comment 17

[Luke Iliffe (Harlequin99)]

When I middle click the last tab, on mouse up I get a second untitled tab appearing in the background then it immediately disappears. The same happens when I use the close tab context menu.

This is on XP using the Jan 16th nightly on a clean profile:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090116 Minefield/3.2a1pre ID:20090116035550

Anyone else getting this, worth filling a new bug?

Comment 19

[Tuukka Tolvanen (sp3000)]

(In reply to comment #17)
> When I middle click the last tab, on mouse up I get a second untitled tab
> appearing in the background then it immediately disappears. The same happens
> when I use the close tab context menu.

Yeah, this tabdancing was mentioned in a bug or two once upon a time, but I don't see one for it specifically.

Comment 20

[Dão Gottwald [:dao]]

(In reply to comment #15)
> (From update of attachment 340733 [details] [diff] [review])
> Either disable or replace with "Close Window" - the latter seems a little nicer
> to me.

Has this comment ever been addressed?

Comment 21

[Marc Auslander]

I know its hopeless - but closing the last tab, replacing it with nothing, is useful function if you have set the option to not close the windows with the last tab.  And, with that option enabled, its completely harmless.

So why take it away?

(why is it useful?  because you don't want to leave the contents of the last tab in view, for whatever reason).

Comment 22

[Tuukka Tolvanen (sp3000)]

> > Either disable or replace with "Close Window" - the latter seems a little nicer
> > to me.
> 
> Has this comment ever been addressed?

That would have been required for the patch that was not taken (attachment 340727 [details] [diff] [review]) which would as such have made the enabled context menu item useless-ui. The taken patch doesn't; it's aimed just at having the window not fall apart when clicked at funny, along the path of least resistance.

The options are switching to a340727 (holy war yay!), or following up with a slightly more gentle last-tab-closeable != closeWindowWithLastTab tweak.

Comment 23

[Tuukka Tolvanen (sp3000)]

...last-tab-closeable == !closeWindowWithLastTab even

Comment 24

[Dão Gottwald [:dao]]

(In reply to comment #22)
> > > Either disable or replace with "Close Window" - the latter seems a little nicer
> > > to me.
> > 
> > Has this comment ever been addressed?
> 
> That would have been required for the patch that was not taken (attachment
> 340727 [details])

It's a direct response to attachment 340733 [details] [diff] [review].

Comment 25

[Dão Gottwald [:dao]]

backed out & taking

I considered filing a new bug, but since patch modified the tabbrowser API, I think it's better to start from scratch.

Comment 26

[Dão Gottwald [:dao]]

Attachment: patch

Comment 27

[Tuukka Tolvanen (sp3000)]

> It's a direct response to attachment 340733 [details] [diff] [review].

Hm, I guess it does look that way.

Why does attachment 357469 [details] [diff] [review] leave out the change to tab-close-button click? Granted, theming it visible on the last tab is not exactly current default configuration, but having it not kill the window seems like a friendly thing to do regardless.

Comment 28

[Dão Gottwald [:dao]]

(In reply to comment #27)
> Why does attachment 357469 [details] [diff] [review] leave out the change to tab-close-button click?
> Granted, theming it visible on the last tab is not exactly current default
> configuration, but having it not kill the window seems like a friendly thing to
> do regardless.

It's not clear to me that if userChrome.css code, an extension or a theme forces the close button to be shown, not closing the window is the expected behaviour. See bug 392870 comment 1. Middle click is the only exception that beltzner mentioned.

Also note that browser.tabs.closeWindowWithLastTab exists for users who want to change that behaviour, not only for the close button but for keyboard shortcuts as well, which seems more consistent.

Comment 29

[:Gavin Sharp [email: [e-mail]]]

I did have second thoughts about adding a public removeClickedTab, so in that respect this patch is better.

I'm not sure I like the idea of special casing only middle-click, though (i.e. having it differ from the context menu and forced-shown tab button cases), given the reasoning in comment 0 (closing the window in response to interaction with the tabbar is unexpected).

At least with the previous patch, the click (as opposed to keyboard) cases were consistent. I suppose not disabling the context menu item leads to an inconsistency with bug 455990 though.

Would people be upset if we disabled both the context menu item and middle clicking? I'm not sure I really understand the "blank out the last tab quickly" use case.

Comment 30

[Marc Auslander]

I'm not sure I really understand the "blank out the last tab quickly"
use case.

you have something on the screen you don't want someone else to see and they arrive.

you have something on the screen you don't want to see, or see any more.

what's not to understand?

Comment 31

[Marc Auslander]

Another case - your one tab has hit a site that's playing audio and you want to kill it without closing the browser.

Comment 32

[Dão Gottwald [:dao]]

Attachment: patch v2

This disables middle clicking unless browser.tabs.closeWindowWithLastTab is set to false.

Comment 34

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/a57fef12385f

Comment 36

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 357544 [details] [diff] [review]
patch v2

a191=beltzner, please make sure to also land the browserchrome test that tests this

Comment 37

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e39a074a5971

Comment 41

[Ken Winiecki, Jr.]

Sorry, what are the resolved behaviors of the tab closebutton and command-w keystroke (File menu / Close Tab item) on a solo tab?

Comment 42

[Dão Gottwald [:dao]]

The tab close button is hidden on the last tab (bug 455990), Cmd-w / File -> Close Tab on the last tab closes the window (bug 392870).

Comment 43

[K. Austers]

I say that this needs to be restored to as it was -- middle click on last open tab can close the entire window. It was very convenient.

Actually, why not make this a checkbox under Preferences/Settings/Options/Whatever-it's-called -> Tabs?

Comment 44

[Dão Gottwald [:dao]]

(In reply to comment #43)
> I say that this needs to be restored to as it was -- middle click on last open
> tab can close the entire window. It was very convenient.

You probably want to file a new bug and request blocking-firefox3.5.

> Actually, why not make this a checkbox under
> Preferences/Settings/Options/Whatever-it's-called -> Tabs?

This would need a new bug, too.

Comment 45

[K. Austers]

(In reply to comment #44)
> (In reply to comment #43)
> > I say that this needs to be restored to as it was -- middle click on last open
> > tab can close the entire window. It was very convenient.
> 
> You probably want to file a new bug and request blocking-firefox3.5.
> 
> > Actually, why not make this a checkbox under
> > Preferences/Settings/Options/Whatever-it's-called -> Tabs?
> 
> This would need a new bug, too.

Ok gonna make it a new bug, thx.

Comment 46

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug's priority relative to the set of other polish bugs is:
P3 - Polish issue that is in a secondary interface, occasionally encountered, or is not easily identifiable.

Effects the primary UI, but assuming that middle click to close isn't that common.

Comment 47

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Looks good on 1.9.2 and 1.9.1. Verified fixed on all platforms with builds like:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b3pre) Gecko/20091112 Namoroka/3.6b3pre ID:20091112034041

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.6pre) Gecko/20091108 Shiretoko/3.5.6pre ID:20091108030959

I have noticed when having the browser.tabs.closeWindowWithLastTab pref set to false there is a short flickering of the close tab button (bug 528597).

Dao, can we use the chrome test which has been already attached to this bug by Martijn?