Remove unnecessary delay when clicking tab close buttons sequentially doesn't work anymore on UX

RESOLVED FIXED in Firefox 27



6 years ago
5 years ago


(Reporter: ge3k0s, Assigned: mconley)



Firefox 27
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [Australis:M7])


(1 attachment)



6 years ago
In latest UX build the behavior introduced in bug 649216 isn't working anymore. You have to click several times to close the tabs sequentially. It may have been caused by bug 851001, but I'm unsure about that.


6 years ago
Blocks: 732583, 649216


6 years ago
Whiteboard: [Australis:M?]

Comment 1

6 years ago
As requested steps to reproduce :
1. Open several tabs
2. Click on the close button and close them sequentially

Expected result : Each time you click on the close button it closes a tab.

Current result : You have to click two times to close each tab.

See Nightly for the correct behavior.
Keywords: regression

Comment 2

6 years ago
Ah, yep, confirmed.

This is because tabbrowser.xml is assuming that the class name on the tab-close-button will be, strictly, "tab-close-button". In reality, it should just ensure that that class exists in the classList.
Assignee: nobody → mconley
Ever confirmed: true

Comment 4

6 years ago
Comment on attachment 765069 [details] [diff] [review]
Patch v1

Look OK, Frank?
Attachment #765069 - Flags: review?(fyan)

Comment 5

6 years ago
Comment on attachment 765069 [details] [diff] [review]
Patch v1

Review of attachment 765069 [details] [diff] [review]:

Thanks! :D

::: browser/base/content/tabbrowser.xml
@@ +4279,5 @@
>           */
>          var clickedOnce = false;
>          function enableDblClick(event) {
>            var target = event.originalTarget;
> +          if (target.classList.contains('tab-close-button'))

Nit: use double quotes for consistency.
I know that the original code used single quotes. That was my bad. :P
Attachment #765069 - Flags: review?(fyan) → review+

Comment 6

6 years ago
Thanks - fixed nit and landed in UX as
Whiteboard: [Australis:M?] → [Australis:M?][fixed-in-ux]
Whiteboard: [Australis:M?][fixed-in-ux] → [Australis:M7][fixed-in-ux]

Comment 7

5 years ago
Should we land this on m-c as well? Seems like a general correctness fix which will also help if anything else (like, say, add-ons) decides to mess with the classes. Asking because this just got me a merge conflict with bug 897751.

Comment 8

5 years ago
Per comment #7, actually setting needinfo this time.
Flags: needinfo?(mconley)
I see no reason to not land this on m-c as well.

Comment 10

5 years ago
Per comment #9,
Flags: needinfo?(mconley)
Whiteboard: [Australis:M7][fixed-in-ux] → [Australis:M7][fixed-in-ux][fixed-in-fx-team]
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M7][fixed-in-ux][fixed-in-fx-team] → [Australis:M7][fixed-in-ux]
Target Milestone: --- → Firefox 27

Comment 12

5 years ago
Whiteboard: [Australis:M7][fixed-in-ux] → [Australis:M7]
You need to log in before you can comment on or make changes to this bug.