Closed Bug 580638 Opened 10 years ago Closed 9 years ago

App tab closes with keystroke

Categories

(Firefox :: Tabbed Browser, enhancement)

enhancement
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: info, Assigned: mano)

References

(Depends on 3 open bugs)

Details

(Whiteboard: [softblocker])

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; Windows NT 5.1; rv:2.0b2pre) Gecko/20100720 Minefield/4.0b2pre
Build Identifier: Mozilla/5.0 (Windows; Windows NT 5.1; rv:2.0b2pre) Gecko/20100720 Minefield/4.0b2pre

As far is i read the App Tab can not be closed by mistake. But this only counts for people handling the mouse, with my keyboard (Ctrl+W) they do close.

I do not know whether this is a bug, but to me it makes the option useless, while i do like the concept of the App Tabs.

Reproducible: Always
maybe a "do you really...?" dialog can be used when closing an app tab with Ctrl-W keystroke
What I also noticed is that when you try to restore your 'mistake' the tab next to the apptab is rendered under the app tab. This is an related bug.
Alex, do we have any specs set for that reported issue?
Status: UNCONFIRMED → NEW
Component: General → Tabbed Browser
Ever confirmed: true
Keywords: uiwanted
OS: Windows XP → All
QA Contact: general → tabbed.browser
Hardware: x86 → All
Version: unspecified → Trunk
Duplicate of this bug: 582519
>uiwanted

the keyboard shortcut should act on the window when only app tabs remain (unless Limi disagrees).  I've been accidentally closing my app tabs a lot, usually when clearing out everything in my tab strip with a quick sequence of command/control-w's

I recommend this blocks, the current behavior breaks the general idea of app tabs being persistent.
blocking2.0: --- → ?
Keywords: uiwanted
I also ran into this bug, personally as someone who really likes the app tab feature and also uses ctrl+w all the time, please make a way that I can say "don't ask me again" if you're going to do a pop-up.
Feedback coming in from all sources is focusing pretty heavily on this bug.  Using a pop-up is too clunky, I recommend we close the window when only app tabs remain (this was the behavior prior to the existence of app tabs, and will be what users expect to have happen).
What about when the user has several normal tabs, and uses ctrl+w on an App Tab?
I personally would like it to switch to the first normal tab.
I do not think closing the window when only app tabs remain will be the best way. You could lose form data.

A dialog with a "do not ask again" check-box will be best in my opinion.

This way someone can choose to do multiple ctrl-w's until the dialog shows up when the app tab is reached, or close them like normal tabs (with the check-box option).
blocking2.0: ? → betaN+
Alex, please see comment 8 and comment 9
Assignee: nobody → mano
Keywords: uiwanted
limi ^ (limi's the Alex on UX driving app tabs)
"Don't ask me again" should be configurable for each application separatly - when I'm closing gmail and I tick "don't ask" checkbox next time it shouldn't ask, but when I close facebook it should ask again
that sounds good for an extension, but sometimes the app is hosted in the root of the host, and sometimes in a directory. which means a lot of custom settings
AppTabs should be un-touchable IMO.  If you want/need to 'close' an AppTab then have it set to revert to a normal tab to close it.  Otherwise AppTabs should be more or less 'forever'.
Since there might be state in the app tabs (actually, more likely than in a normal tab), we shouldn't let these close when you use Cmd-W or equivalent. So I'd say that closing the whole window would be even more confusing.

The options are probably: a) don't allow closing of app tabs using keyboard shortcuts, or b) warn when you're about to close an app tab.

Since it seems weird to me that you can't close app tabs from the keyboard, I'm leaning towards a warning asking "do you really want to close this app tab", with a "do not ask me again" checkbox, so people that don't mind can do it without the prompt from there on.

In general, I don't like those kinds of questions, but I think it might be warranted here. We should make sure we don't get double questions here (ie. if it has an onunload handler that asks, we shouldn't)
By default every profile is going to have one app tab as the home tab.  This takes us towards the modal dialog box that IE has on exit for every time the user tries to dismiss the window using a keyboard shortcut.
Thinking about it some more (and discussing with faaborg), maybe we should try with the following behavior:

- If you're trying to close an app tab with a keyboard shortcut *when there are other normal tabs open*, we should ask if you try to close the app tab using a keyboard shortcut. If you select via the context menu, we just do it, since it's less likely to be accidental.

- If app tabs are the only ones left — ie. *no normal tabs exist* in that window, it's unlikely that the keyboard close operation is accidental, and we just close the window (which may imply Exit on Windows if it was the last window, but not on Mac). We do respect things like onunload handlers for app tabs, though — so if there's data on the page that can be lost, the dialog that takes care of that should still work like it currently does.
Note: Currently, App Tabs don't get restored when you open a new window, this is still missing functionality. So when you close your window and open a new one, the app tabs should be there, and persist between windows. 

Somewhat unrelated to this particular issue, but might help with understanding why closing the window isn't that big of a deal when this is implemented.
What's the bug # for that change?
Apptabs close on middle click as well. FWIW, I can also confirm the rendering glitch in Comment 2.
I don't think we can block Firefox 4 on this; there's a bunch of edge cases that need to be specified in terms of behaviour, and any additional confirmation dialogs are now past-due on strings.
blocking2.0: betaN+ → -
I don't think we need a confirmation dialog box, we should just dismiss the window when only app tabs remain, and if there are other tabs open in the window, do nothing.
That seems pretty destructive to me; why do you think that users are going to expect that App Tabs wouldn't close like any other tab? Especially since they presently behave like other tabs in all other ways.
Sorry to throw my mostly un-informed hat into the ring. But if App Tabs "presently behave like other tabs in all other ways", why do they exist?

I thought they were supposed to be persistent items that were always there whenever you are browsing so that you don't have to worrying about reopening the tab later or resetting the app tab.

Often times I want to "start over" so i hit ctrl+w a bunch to "start over". Now I have accidentally closed my app tabs, which are a pain to create (open tabs, go to the website, right click select, repeat for each app tab), but apparently simple to destroy (ctrl+w).

I know i could learn to use ctrl+shift+w to close the window instead of ctrl+w over and over, and I'm not sure how many users use ctrl+w to close tabs, but that's my use case and while I chimed in on this bug.
>why do you think that users are going to
>expect that App Tabs wouldn't close like any other tab?

The nature of an app tab is to remain open, to close an app tab is to go against its purpose.  They should be just as hard to dismiss as they are to create.  If users have varying exceptions, their expectations will change when they figure out how app tabs behave.
Blocks: 601565
Blocks: 579874
(In reply to comment #24)
> Often times I want to "start over" so i hit ctrl+w a bunch to "start over". Now
> I have accidentally closed my app tabs, which are a pain to create (open tabs,
> go to the website, right click select, repeat for each app tab), but apparently
> simple to destroy (ctrl+w).

We'd better make it more easy to recreate an apptab. When a tab is apptabed, Firefox should remember this so that when it is closed and reopened, it is automatically apptabed.
On twitter beltzner tweets:

>@faaborg @shaver thought I denied blocking on having entire window close when >you ctrl+w with only app tabs left. Disabling ctrl+w is right.

Renominating to make sure we disable the ability to quickly close (accel-w, middle click).  The effect on the window can be figured out in a different bug.
blocking2.0: - → ?
Summary: App Tab closes with keystroke → App Tab closes with keystroke or middle click
(In reply to comment #26)
> We'd better make it more easy to recreate an apptab. When a tab is apptabed,
> Firefox should remember this so that when it is closed and reopened, it is
> automatically apptabed.

Undo Close Tab aka Recently Closed Tabs in your history menu will restore an app tab with state. It will make that tab an app tab again.
Blocking+ for comment #27. First do no harm.
blocking2.0: ? → final+
Since we have no session persistence if you close the window — ie. if we're going to close the window when there are only app tabs left — I don't think that's an option.

Recommended behavior:
* Disable Cmd/Ctrl-W (and any other "close tab" shortcuts) for app tabs. If you close a row of tabs by quickly tapping Cmd-W a number of times, it should stop closing tabs when there are only app tabs left
* Just for completeness: Even if there are normal tabs available, they should not close if you have an app tab active.
* "Close other tabs" in the context menu on tabs should only close normal tabs, not app tabs.
* No dialogs, no special behavior outside of this.
* If you want to close an app tab, you use the mouse.
* We could add something like Shift-Cmd/Ctrl-W if we think it needs a keyboard shortcut, but this should be done in a separate bug, as mentioned.
(In reply to comment #30)
> Shift-Cmd/Ctrl-W

closes the window.
(In reply to comment #31)
> (In reply to comment #30)
> > Shift-Cmd/Ctrl-W
> 
> closes the window.

Yup, just added as an example. Alt/Opt might be a better qualifier (it's mainly to give people a way to do it if they are heavy users of it — it doesn't need to be very discoverable as such)
A different shortcut key seems inconsistent. Maybe a hidden preference like "browser.tabs.closeAppTabs"?
blocking2.0: final+ → betaN+
Duplicate of this bug: 621872
The recommendation here is still the same as in comment 30:

* Disable Cmd/Ctrl-W (and any other "close tab" shortcuts) for app tabs. If you
close a row of tabs by quickly tapping Cmd-W a number of times, it should stop
closing tabs when there are only app tabs left
* "Close other tabs" in the context menu on tabs should only close normal tabs,
not app tabs.
* Just for completeness: Even if there are normal tabs available, they should
not close if you have an app tab active.
* No dialogs, no special behavior outside of this.
* If you want to close an app tab, you use the mouse.
* We could add something like Opt/Alt-Cmd/Ctrl-W if we think it needs a keyboard
shortcut, but this should be done in a separate bug, as mentioned.
* If you close the window using Shift-Cmd-W, the app tabs go with it (at least until they are globally persistent, which isn't happening for Firefox 4.0).

(In reply to comment #33)
> A different shortcut key seems inconsistent. Maybe a hidden preference like
> "browser.tabs.closeAppTabs"?

That works for me, I'll leave that up to the implementer(s) if they want to add support for a preference.
Keywords: uiwanted
Duplicate of this bug: 624608
Whiteboard: [softblocker]
Comment 35 makes sense to me, completely. Happily, "Close other tabs" already works as specified there.
Just to check that the proposal in comment 35 also means that on OSX cmd+w closes the window when only app tabs are left.
(In reply to comment #38)
> Just to check that the proposal in comment 35 also means that on OSX cmd+w
> closes the window when only app tabs are left.

No. Maybe when app tabs persist across all windows, and return when you open a new one, but we don't do that right now, so we shouldn't close a window that has app tabs using Cmd-W.
Hmm, that means that on OSX there is no keyboard shortcut to close a window without shutting down Firefox anymore.
I would call that a bug.
(In reply to comment #40)
> Hmm, that means that on OSX there is no keyboard shortcut to close a window
> without shutting down Firefox anymore.
> I would call that a bug.

There's currently no way to create an app tab without a mouse either, so they are mutually exclusive.

(note that I'm not saying this is how app tabs should ideally function — because it isn't — but the app tabs we're shipping with 4.0 are quite limited compared to the original intent)
(In reply to comment #41)
> (In reply to comment #40)
> > Hmm, that means that on OSX there is no keyboard shortcut to close a window
> > without shutting down Firefox anymore.
> > I would call that a bug.
> 
> There's currently no way to create an app tab without a mouse either

The tab context menu can be opened with the keyboard.
(In reply to comment #42)
> (In reply to comment #41)
> > (In reply to comment #40)
> > > Hmm, that means that on OSX there is no keyboard shortcut to close a window
> > > without shutting down Firefox anymore.
> > > I would call that a bug.

Shift+Cmd+W doesn't work?

> > There's currently no way to create an app tab without a mouse either
> 
> The tab context menu can be opened with the keyboard.

Not on OS X.
(In reply to comment #41)
> (In reply to comment #40)
> > Hmm, that means that on OSX there is no keyboard shortcut to close a window
> > without shutting down Firefox anymore.
> > I would call that a bug.
> 
> There's currently no way to create an app tab without a mouse either, so they
> are mutually exclusive.

Well, that is true of course. But closing tabs and windows is a far more frequently occurring operation than creating an app tab and so a keyboard shortcut for these actions is most appreciated.

Furthermore in the current setup it is also possible to close the only window with app tabs by (accidentally) clicking the close button with the mouse.
Granted that is far less likely to occur but if the primary reason for excluding this functionality is to prevent users from closing their app tabs irrecoverably than these problems are somewhat equivalent.

The solution to both would of course be to simply have the app tabs restored when a new window is created if no other active window already has them.
If the user then closes the window containing the app tabs by accident he/she only has to open a new window which feels natural.

Although this may be more difficult if different windows can have different app tabs.
 
> (note that I'm not saying this is how app tabs should ideally function —
> because it isn't — but the app tabs we're shipping with 4.0 are quite limited
> compared to the original intent)
(In reply to comment #43)
> (In reply to comment #42)
> > (In reply to comment #41)
> > > (In reply to comment #40)
> > > > Hmm, that means that on OSX there is no keyboard shortcut to close a window
> > > > without shutting down Firefox anymore.
> > > > I would call that a bug.
> 
> Shift+Cmd+W doesn't work?

Yes it does, however this changes standard application behavior on OSX (at least for all applications I use).
Normally any of the following happens when I enter Cmd+W

1. Current tab closes
2. Current window closes (if no tabs are left)
3. Application closes (for applications that don't have tabs such as iPhoto for example)

This would add a 4th option to the list of possible behavior.

4. Nothing happens (only app tabs are left in current window)

I'm not sure what Apple's human interface guidelines say about this (not even sure if they exist) but I'm not a big fan of introducing new behavior this way.

There is an even bigger problem with this I'm afraid.
If the window containing the app tabs is the last one open and I close it with shift+cmd+w it works as desired (e.g. cmd+n opens a new window with app tabs, cmd+q followed by a restart does the same).
However if it is not the last window open then I lose all my app tabs (e.g. cmd+n gives me a new window without app tabs and so does a restart).
This is probably a separate bug though and I have not checked if it has already been reported elsewhere.

> 
> > > There's currently no way to create an app tab without a mouse either
> > 
> > The tab context menu can be opened with the keyboard.
> 
> Not on OS X.
We need some kind of determination on this bug. It's a little late in the game to still refining behavior of a new feature, and if we are going to change it, we need a little traction.
(In reply to comment #46)
> We need some kind of determination on this bug. It's a little late in the game
> to still refining behavior of a new feature, and if we are going to change it,
> we need a little traction.

We're not refining, and the the expected behavior is still what was recommended in comment 30, and repeated in comment 35. I don't know how much clearer I can make it. :)
Sure, I understand the pressure to release is on.
I am also much more happy with the behavior suggested in comment 35 than the current behavior in Beta 9.

Furthermore I have searched bugzilla and found that one of the major concerns I have with app tab behavior is already reported many times (e.g. bug 587400, 594831, etc.) so this is a separate issue.

Perhaps it would be a good idea to have one tracking bug to outline desired app tab behavior on all platforms (if this does not already exist).
This would include keyboard shortcuts, app tab persistence, visibility in multiple windows, grouping of app tabs in panorama, possible desktop shortcuts, undocking app tabs into a separate window, ...
Just a thought :)

Anyway, I guess its not too bad to have to use Cmd + Shift + W to close the window for now.
(In reply to comment #43)
> (In reply to comment #42)
> > (In reply to comment #41)
> > > (In reply to comment #40)
> > > There's currently no way to create an app tab without a mouse either
> > 
> > The tab context menu can be opened with the keyboard.
> 
> Not on OS X.

Actually, if you select the tab over to the tab bar, control+space opens up the context menu.
> * "Close other tabs" in the context menu on tabs should only close normal tabs,
> not app tabs.

That's the way it works on current trunk, afacit.  We also make sure to disable it if there are no normal tabs in the tabstrip.

> * Just for completeness: Even if there are normal tabs available, they should
> not close if you have an app tab active.

I'm not sure I get this. Do you mean that Close Other Tabs should always be disabled when an App Tab is selected?
The tricky part here is updating File > Close.  Some xul bugs are in the way - All of which are worked-around in this patch.

One bug remains: pressing Cmd+W on mac would flashes the File menu. I filed bug 630839 for the underlying issue and bug 630840 for a bug that blocks a work around in browser/.
Attachment #509081 - Flags: review?
Attachment #509081 - Flags: review? → review?(dietrich)
Forgot to ask: what's the decision for middle click?
(In reply to comment #50)
> > * Just for completeness: Even if there are normal tabs available, they should
> > not close if you have an app tab active.
> 
> I'm not sure I get this. Do you mean that Close Other Tabs should always be
> disabled when an App Tab is selected?

No — Close Other Tabs should close normal tabs even when an app tab is selected, but not other app tabs.

What I was trying to clarify: if you have an app tab selected, and hit Ctrl-W repeatedly, it shouldn't start closing any of the normal tabs. Not sure anyone would expect that, but just wanted to make that 100% clear. And it seems I made it more confusing instead. ;)

(In reply to comment #52)
> Forgot to ask: what's the decision for middle click?

The consensus seems to be that since you need to aim when you use middle click, it's different from mashing the Ctrl-W keyboard shortcut repeatedly. Furthermore, middle-click seems to be a classic power-user feature, so I'd just leave it to work as it currently does.
(In reply to comment #53)

> No — Close Other Tabs should close normal tabs even when an app tab is
> selected, but not other app tabs.
> 

<Mano>	limi: that's the current behavior

> What I was trying to clarify: if you have an app tab selected, and hit Ctrl-W
> repeatedly, it shouldn't start closing any of the normal tabs. Not sure anyone
> would expect that, but just wanted to make that 100% clear. And it seems I made
> it more confusing instead. ;)
> 

<Mano>	limi: The use case you've described cannot happen. If i have an app tab selected, then accel+w is disabled
<Mano>	no matter how many times you press it, no matter how hard you do so.

> (In reply to comment #52)
> > Forgot to ask: what's the decision for middle click?
> 
> The consensus seems to be that since you need to aim when you use middle click,
> it's different from mashing the Ctrl-W keyboard shortcut repeatedly.
> Furthermore, middle-click seems to be a classic power-user feature, so I'd just
> leave it to work as it currently does.

OK!
Comment on attachment 509081 [details] [diff] [review]
When an app tab is selected, disable Accel+W and remove the keyboard shortcut from File > Close Tab

aside from needing a test, this looks ok to me. i don't know this code well enough though, so please get review from someone who regularly does tabbrowser reviews.
Attachment #509081 - Flags: review?(dietrich) → feedback+
Attached patch With a testSplinter Review
Attachment #509081 - Attachment is obsolete: true
Attachment #510250 - Flags: review?(dao)
Comment on attachment 510250 [details] [diff] [review]
With a test

I'd call the method _setCloseKeyState rather than _updateCloseKeyState.

>+          // Fixing bug 630826 could make that happen automatically.
>+          // Fixing bug 630830 could avoid the ugly hack below.
>+          
>+          let closeMenuItem = document.getElementById("menu_close");
>+          let parentPopup = closeMenuItem.parentNode;
>+          let nextItem = closeMenuItem.nextSibling;
>+          let clonedItem = closeMenuItem.cloneNode(true);
>+
>+          parentPopup.removeChild(closeMenuItem);

Bug 630830 has a patch, maybe we can wait for that.

>+          if (aEnabled)
>+            clonedItem.setAttribute("key", "key_close");
>+          else
>+            clonedItem.removeAttribute("key");
>+          

trailing spaces on the last line

>+    if (aPinned) {
>+      ok(elemAttr("key_close", "disabled") == "true",
>+         "key_close should be disabled when a piined-tab is selected");

ok(a == b, c) should be is(a, b, c)

"piined" is misspelled

>+  let pinnedTab = gBrowser.loadOneTab("about:blank");
>+  gBrowser.pinTab(pinnedTab);
>+
>+  // Just pinning the tab shouldn't change the key state.
>+  testState(false);

addTab instead of loadOneTab would be more explicit, since its behavior doesn't depend on browser.tabs.loadInBackground.
Attachment #510250 - Flags: review?(dao) → review+
> Bug 630830 has a patch, maybe we can wait for that.

Unfortunately, the patch there won't fix mac menus.
http://hg.mozilla.org/mozilla-central/rev/88952ceddd05
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Mozilla/5.0 (X11; Linux i686; rv:2.0b12) Gecko/20100101 Firefox/4.0b12

Verified issue.
Status: RESOLVED → VERIFIED
Summary: App Tab closes with keystroke or middle click → App tab closes with keystroke
Blocks: 637486
No longer blocks: 601565
No longer blocks: 579874
Verified fixed on Mozilla/5.0 (Windows NT 6.1; rv:2.2a1pre) Gecko/20110404 Firefox/4.2a1pre
Depends on: 648488
You need to log in before you can comment on or make changes to this bug.