Closed Bug 880277 Opened 10 years ago Closed 9 years ago

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


(Firefox :: Tabbed Browser, defect)

Not set



Firefox 27


(Reporter: u428464, Assigned: mconley)



(Keywords: regression, Whiteboard: [Australis:M7])


(1 file)

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.
Whiteboard: [Australis:M?]
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
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
Attached patch Patch v1Splinter Review
Comment on attachment 765069 [details] [diff] [review]
Patch v1

Look OK, Frank?
Attachment #765069 - Flags: review?(fyan)
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+
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]
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.
Per comment #7, actually setting needinfo this time.
Flags: needinfo?(mconley)
I see no reason to not land this on m-c as well.
Per comment #9,
Flags: needinfo?(mconley)
Whiteboard: [Australis:M7][fixed-in-ux] → [Australis:M7][fixed-in-ux][fixed-in-fx-team]
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M7][fixed-in-ux][fixed-in-fx-team] → [Australis:M7][fixed-in-ux]
Target Milestone: --- → Firefox 27
Whiteboard: [Australis:M7][fixed-in-ux] → [Australis:M7]
You need to log in before you can comment on or make changes to this bug.