649316 - allow closing tabs in panorama with del and cmd+backspace

Status: RESOLVED WONTFIX

(Firefox Graveyard :: Panorama, defect)

Description

[eyal gruss]

new tab and undo close tab keyboard shortcuts work in panorama. close tab should work as well for the focused tab. this is useful when closing multiple tab.

Comment 1

[Ian Gilman [:iangilman]]

As discussed in bug 579199, I think ctrl+w is a confusing key combo for this feature. I recommend the delete key instead.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

bugspam

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

bugspam

Comment 4

[Raymond Lee [:raymondlee]]

(In reply to comment #1)
> As discussed in bug 579199, I think ctrl+w is a confusing key combo for this
> feature. I recommend the delete key instead.

Some mac machines e.g. macbook / macbook pro don't have the delete key so cmd+backspace does the same thing.

Comment 5

[Raymond Lee [:raymondlee]]

Attachment: v1

Handle two keys in Panorama to remove tab item
* delete key
* backspace key (following bookmark manager which user can use backspace to remove bookmark, instead of cmd+backspace)

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 542750 [details] [diff] [review]
v1

Review of attachment 542750 [details] [diff] [review]:
-----------------------------------------------------------------

I like that you want to be consistent with the bookmark manager shortcuts but I'm a bit concerned about the backspace key removing tabs as I'm using that "Go Back" in history. That is default on windows systems I think and may yield some really unexpected behavior. Maybe we should ask the UX team to chime in. Anyway, the patch looks good!

::: browser/base/content/test/tabview/browser_tabview_bug649316.js
@@ +8,5 @@
> +  gBrowser.addTab();
> +  gBrowser.addTab();
> +
> +  registerCleanupFunction(function() {
> +    hideTabView(function() {});

Nit: please add some code that removes the new tabs if for some reason the other part fails to do so.

Comment 7

[Raymond Lee [:raymondlee]]

Attachment: v2

Add code to remove tabs in registerCleanupFunction

Comment 8

[Ian Gilman [:iangilman]]

Comment on attachment 542756 [details] [diff] [review]
v2

Review of attachment 542756 [details] [diff] [review]:
-----------------------------------------------------------------

Beautiful. 

We do need UX to weigh in on the backspace. I personally like the idea of following the backspace usage in the bookmark manager, but then I don't really care for backspace as "back" key. I can imagine, however, that for people who use the backspace key as their primary "back" key it would be quite jarring to enter Panorama and decide you wanted to "go back" from it, hit the backspace, and instead lose the tab you had been looking at. For that reason, command+backspace (control+backspace on Windows/Linux?) seems like the safest route (in addition to the delete key, of course). 

Faaborg, your thoughts?

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

As a quick idea, we could allow only "backspace" to remove a tab when ("browser.backspace_action" not in [0, 1]) - http://kb.mozillazine.org/Browser.backspace_action

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

bugspam

(Fx7 was branched, removing open bugs from Fx7 meta bug, we won't create new meta bugs for upcoming Fx versions)

Comment 11

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

Comment on attachment 542756 [details] [diff] [review]
v2

I want to avoid overloading keys because of the possible mode errors.  Let's say we use the same keyboard shortcuts both in normal view and in tab view.  Then for instance, you expect backspace or delete to clear text when you are in a text field, but since it wasn't focused anymore you end up closing the tab.

The main problem people had with command-w in panorama was that they expected it to close the view itself, but hopefully we've started to address that with the view not looking like it is itself inside of a tab.

Comment 12

[Raymond Lee [:raymondlee]]

Based on comment 11, closing this bug.