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

RESOLVED FIXED in Firefox 27

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ge3k0s, Assigned: mconley)

Tracking

({regression})

Trunk
Firefox 27
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:M7])

Attachments

(1 attachment)

(Reporter)

Description

5 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.
(Reporter)

Updated

5 years ago
Blocks: 732583, 649216

Updated

5 years ago
Whiteboard: [Australis:M?]
(Reporter)

Comment 1

5 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
(Assignee)

Comment 2

5 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
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 4

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

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

Comment 5

5 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+
(Assignee)

Comment 6

5 years ago
Thanks - fixed nit and landed in UX as https://hg.mozilla.org/projects/ux/rev/bc8b83f7a57e
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, https://hg.mozilla.org/integration/fx-team/rev/1c0baa3cf12c
Flags: needinfo?(mconley)
Whiteboard: [Australis:M7][fixed-in-ux] → [Australis:M7][fixed-in-ux][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/1c0baa3cf12c
Status: ASSIGNED → RESOLVED
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
https://hg.mozilla.org/mozilla-central/rev/bc8b83f7a57e
Whiteboard: [Australis:M7][fixed-in-ux] → [Australis:M7]
You need to log in before you can comment on or make changes to this bug.