Closed Bug 649316 Opened 12 years ago Closed 11 years ago

allow closing tabs in panorama with del and cmd+backspace

Categories

(Firefox Graveyard :: Panorama, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: eyalgruss, Assigned: raymondlee)

Details

Attachments

(1 file, 1 obsolete file)

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.
Blocks: 603789
As discussed in bug 579199, I think ctrl+w is a confusing key combo for this feature. I recommend the delete key instead.
Summary: allow closing tabs in panorama with ctrl+w → allow closing tabs in panorama with keyboard shortcut (ctrl+w or del)
No longer blocks: 603789
bugspam
No longer blocks: 653099
bugspam
Blocks: 660175
(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.
Summary: allow closing tabs in panorama with keyboard shortcut (ctrl+w or del) → allow closing tabs in panorama with del and cmd+backspace
Attached patch v1 (obsolete) — Splinter Review
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)
Assignee: nobody → raymond
Status: NEW → ASSIGNED
Attachment #542750 - Flags: feedback?
Attachment #542750 - Flags: feedback? → feedback?(tim.taubert)
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.
Attachment #542750 - Flags: feedback?(tim.taubert) → feedback+
Attached patch v2Splinter Review
Add code to remove tabs in registerCleanupFunction
Attachment #542750 - Attachment is obsolete: true
Attachment #542756 - Flags: ui-review?(faaborg)
Attachment #542756 - Flags: review?(ian)
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?
Attachment #542756 - Flags: review?(ian) → review+
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
bugspam

(Fx7 was branched, removing open bugs from Fx7 meta bug, we won't create new meta bugs for upcoming Fx versions)
No longer blocks: 660175
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.
Attachment #542756 - Flags: ui-review?(faaborg) → ui-review-
Status: ASSIGNED → NEW
Based on comment 11, closing this bug.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.