986920 - [Windows Classic] New tab button stealing focus from unselected tabs

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Tss]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140322030204

Steps to reproduce:

Using Windows Classic theme, when you hover over the rightmost unselected tab (next to the new tab button), new tab button takes focus when the cursor is still over the unselected tab's close button.

Bug 978752 - "Tab strip gradient is broken in customization mode on Windows XP in classic mode" introduced some z-index layering to tabs and new tab button, and I think that's the origin of the problem. I used

main-window[tabsintitlebar]:not([sizemode=fullscreen]) .tabbrowser-tab:not([selected=true]) {
    z-index: 2 !important;
    }

#main-window[tabsintitlebar]:not([sizemode=fullscreen]) .tabbrowser-tab[selected=true] {
    z-index: 3 !important;
    }

to set the tabs on higher layers, and it seems to fix the issue.

Comment 1

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

Attachment: Patch v1

Naive patch. Should I put some space between these z-index's for add-ons?

Comment 2

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

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

Nope nope nope - this overlaps the selected tab.

Comment 3

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

Attachment: Patch v2

So, this appears to be better. Layering seems to be right. Kept my eye on things like the separators and the overflow arrow buttons, and didn't see anything hairy.

As before, I wonder if I should space out the indexes a little bit for add-ons to jam things in between. Open to suggestions there.

Comment 4

[:Gijs (he/him)]

Comment on attachment 8396602 [details] [diff] [review]
Patch v2

Review of attachment 8396602 [details] [diff] [review]:
-----------------------------------------------------------------

WFM.

Comment 5

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

Thanks!

remote:   https://hg.mozilla.org/integration/fx-team/rev/eefdd02dd206

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[:Gijs (he/him)]

Comment on attachment 8396602 [details] [diff] [review]
Patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis! But also, bug 978752
User impact if declined: weird overlap between tabs and new tab button
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): relatively low, just changes z-indices of the tabs
String or IDL/UUID changes made by this patch: none

Comment 8

[Dão Gottwald [:dao]]

This code needs some documentation. It's not even obvious why .tabs-newtab-button has z-index:1 in the first place, let alone that this is what made you give .tabbrowser-tab a z-index too.

Comment 9

[Mike de Boer [:mikedeboer]]

Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/9fc38ffaff75

Comment 10

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

(In reply to Dão Gottwald [:dao] from comment #8)
> This code needs some documentation. It's not even obvious why
> .tabs-newtab-button has z-index:1 in the first place, let alone that this is
> what made you give .tabbrowser-tab a z-index too.

Sure, I can add that documentation. Would you mind filing a bug and assigning it to me?

Comment 11

[Mike de Boer [:mikedeboer]]

Beta: https://hg.mozilla.org/releases/mozilla-beta/rev/e24101b952ad

Comment 12

[Bogdan Maris, Desktop Test Engineering]

Reproduced the initial issue on nightly build from 2014-03-23, verified fixed on Windows XP 32bit, Windows 7 32bit/64bit using Firefox 29 beta 4, latest Aurora and latest Nightly.

Comment 13

[mcdavis941 (sporadically reading bugmail)]

It looks like this should be the patch for beta (rather than e24101b952ad):

https://hg.mozilla.org/releases/mozilla-beta/rev/126e650100c6

Comment 14

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

Yes, that's correct - thanks mcdavis941.

Comment 15

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

Backed out of mozilla-central as https://hg.mozilla.org/integration/fx-team/rev/6a5529e2150d

We landed a better solution in Bug 989761. Back-outs for mozilla-aurora and mozilla-beta will follow once bug 989761 gets uplift approval.

Comment 16

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

Ugh - I should have said backed out of fx-team, not mozilla-central. Of course this will get backed out of mozilla-central after merge.

Comment 17

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

Backouts:

mozilla-central: https://hg.mozilla.org/mozilla-central/rev/6a5529e2150d

Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/a9b6b1a08970

Beta: https://hg.mozilla.org/releases/mozilla-beta/rev/a90a4219b520


This problem was fixed with a different solution in bug 989761.