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

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[u428464]

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.

Comment 1

[u428464]

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.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

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.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 765069 [details] [diff] [review]
Patch v1

Look OK, Frank?

Comment 5

[Frank Yan (:fryn)]

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

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Thanks - fixed nit and landed in UX as https://hg.mozilla.org/projects/ux/rev/bc8b83f7a57e

Comment 7

[:Gijs (he/him)]

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

[:Gijs (he/him)]

Per comment #7, actually setting needinfo this time.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

I see no reason to not land this on m-c as well.

Comment 10

[:Gijs (he/him)]

Per comment #9, https://hg.mozilla.org/integration/fx-team/rev/1c0baa3cf12c

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1c0baa3cf12c

Comment 12

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/bc8b83f7a57e