Closed Bug 456382 Opened 12 years ago Closed 11 years ago

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

Categories

(Firefox :: Tabbed Browser, enhancement)

3.5 Branch
enhancement
Not set

Tracking

()

VERIFIED FIXED
Firefox 3.6a1

People

(Reporter: tuukka.tolvanen, Assigned: dao)

References

Details

(Keywords: polish, verified1.9.1, Whiteboard: [polish-easy] [polish-interactive][polish-p3])

Attachments

(2 files, 6 obsolete files)

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.
This is a fall-out from bug 392870. See bug 455852 and bug 455990
Attached patch patch1 (obsolete) — Splinter Review
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.
Assignee: nobody → tuukka.tolvanen
Status: NEW → ASSIGNED
Attachment #339891 - Flags: review?
Attached patch patch2 (obsolete) — Splinter Review
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.
Attachment #339891 - Attachment is obsolete: true
Attachment #339898 - Flags: review?(gavin.sharp)
Attachment #339891 - Flags: review?
This interferes with the patch in bug 456002.
updated for bug 456002 attachment 339788 [details] [diff] [review]
Attachment #339898 - Attachment is obsolete: true
Attachment #339905 - Flags: review?(gavin.sharp)
Attachment #339898 - Flags: review?(gavin.sharp)
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.
Blocks: 455852
Blocks: 456405
No longer blocks: 455852
It needs to be decided if we do this and bug 455990 or bug 456425.
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.
Severity: normal → enhancement
Summary: Middle click on last tab closes window → Middle click on last tab shouldn't close window
OS: Linux → All
Hardware: PC → All
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.
Summary: Middle click on last tab shouldn't close window → Middle click or close-button on last tab shouldn't close window
Depends on: 456425
Keywords: uiwanted
You'll probably have a better chance to get this ui-reviewed if you make middle clicking a no-op for the last tab.
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.
(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.
Attached patch patchB1, the no-op edition (obsolete) — Splinter Review
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.
Attached patch patchA4, cover context-close too (obsolete) — Splinter Review
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.
Attachment #339905 - Attachment is obsolete: true
Attachment #340733 - Flags: ui-review?(beltzner)
Attachment #340733 - Flags: review?(gavin.sharp)
Attachment #339905 - Flags: review?(gavin.sharp)
Summary: Middle click or close-button on last tab shouldn't close window → mousing on last tab shouldn't close window (middle click, close button, context menu)
Attachment #340727 - Flags: ui-review?(beltzner)
No longer depends on: 456425
Flags: blocking-firefox3.1?
Attachment #340727 - Flags: ui-review?(beltzner) → ui-review+
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.
Attachment #340733 - Flags: ui-review?(beltzner) → ui-review+
Flags: wanted-firefox3.1+
Flags: blocking-firefox3.1?
Flags: blocking-firefox3.1-
Keywords: uiwanted
Whiteboard: [needs review gavin]
Attachment #340733 - Flags: review?(gavin.sharp) → review+
Whiteboard: [needs review gavin]
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/e30f1765a42c
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.2a1
Attachment #340733 - Flags: approval1.9.1?
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?
Duplicate of this bug: 465114
(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.
(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?
Attachment #340733 - Flags: approval1.9.1?
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).
> > 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.
...last-tab-closeable == !closeWindowWithLastTab even
(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].
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.
Assignee: tuukka.tolvanen → dao
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch patch (obsolete) — Splinter Review
Attachment #340727 - Attachment is obsolete: true
Attachment #340733 - Attachment is obsolete: true
Attachment #357469 - Flags: review?(gavin.sharp)
Status: REOPENED → ASSIGNED
Keywords: polish
Whiteboard: [polish-easy] [polish-interactive]
Version: Trunk → 3.1 Branch
> 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.
(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.
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.
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?
Another case - your one tab has hit a site that's playing audio and you want to kill it without closing the browser.
Attached patch patch v2Splinter Review
This disables middle clicking unless browser.tabs.closeWindowWithLastTab is set to false.
Attachment #357469 - Attachment is obsolete: true
Attachment #357544 - Flags: review?(gavin.sharp)
Attachment #357469 - Flags: review?(gavin.sharp)
Target Milestone: Firefox 3.2a1 → ---
Duplicate of this bug: 475155
Attachment #357544 - Flags: review?(gavin.sharp) → review+
http://hg.mozilla.org/mozilla-central/rev/a57fef12385f
Status: ASSIGNED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.2a1
Attachment #357544 - Flags: approval1.9.1?
Duplicate of this bug: 483173
Summary: mousing on last tab shouldn't close window (middle click, close button, context menu) → mousing on last tab shouldn't close window (middle click, context menu)
Comment on attachment 357544 [details] [diff] [review]
patch v2

a191=beltzner, please make sure to also land the browserchrome test that tests this
Attachment #357544 - Flags: approval1.9.1? → approval1.9.1+
Flags: in-testsuite?
Keywords: checkin-needed
Duplicate of this bug: 486317
Duplicate of this bug: 474504
Duplicate of this bug: 489003
Sorry, what are the resolved behaviors of the tab closebutton and command-w keystroke (File menu / Close Tab item) on a solo tab?
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).
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?
(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.
(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.
Depends on: 486393
Depends on: 491258
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.
Whiteboard: [polish-easy] [polish-interactive] → [polish-easy] [polish-interactive][polish-p3]
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?
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.