:Gijs (he/him) Reporter
	Description • 8 years ago

Potentially related to bug 1178909, and/or maybe a regression from bug 1308407?

	Jeff Griffiths (:canuckistani) (:⚡︎)
	Comment 1 • 7 years ago

Gijs, I don't have a win7 test vm, can you still repro this? If so, it should block compact themes.

Setting P1 for now.

	:Gijs (he/him) Reporter
	Comment 2 • 7 years ago

Yes, this still reproduces.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 3 • 7 years ago

I would like to help with this issue if possible...  Gijs, I currently only have Windows 10 systems.  Is there any way I can force Firefox to use the Windows 7 theme for Windows 10, or is a Windows 7 VM the only way?

	:Gijs (he/him) Reporter
	Comment 4 • 7 years ago

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #3)
> I would like to help with this issue if possible...  Gijs, I currently only
> have Windows 10 systems.  Is there any way I can force Firefox to use the
> Windows 7 theme for Windows 10,

Not without inverting a load of media queries and updating a lot of jar.mn overrides.

> or is a Windows 7 VM the only way?

That is certainly simpler.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 7 • 7 years ago

I have attached what I see in the tab for Windows 10 vs. Windows 7 with Compact Light enabled.  The three tabs show tab close on hover, unselected tab, and selected tab.

Gijs, what exactly is incorrect in Windows 7 here?  (I think I am just not familiar with what's expected...)  The overall coloring seems similar between the two...  The main difference I could see is that on Windows 7, when you hover you get a red background square with white X, while Windows 10 has a dark gray circle with white X.

	:Gijs (he/him) Reporter
	Comment 8 • 7 years ago

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #7)
> I have attached what I see in the tab for Windows 10 vs. Windows 7 with
> Compact Light enabled.  The three tabs show tab close on hover, unselected
> tab, and selected tab.
> 
> Gijs, what exactly is incorrect in Windows 7 here?  (I think I am just not
> familiar with what's expected...)  The overall coloring seems similar
> between the two...  The main difference I could see is that on Windows 7,
> when you hover you get a red background square with white X, while Windows
> 10 has a dark gray circle with white X.

This is fine. The problem is when you use Windows Classic (with the default dark blue titlebar) rather than aero basic/lite (which seems to be what's in your screenshot). Here's a screenshot of what I see with Windows Classic: http://imgur.com/a/Mltmz .

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 9 • 7 years ago

Thanks, I see the issue now.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 11 • 7 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e085062bf846e0cbc95be05c7226a154939f9a43

	:Gijs (he/him) Reporter
	Comment 12 • 7 years ago

I haven't tested, but off-hand it looks like your added rules apply to the dark lwtheme as well, and there I think we do want the inverted icons?

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 13 • 7 years ago

(In reply to :Gijs from comment #12)
> I haven't tested, but off-hand it looks like your added rules apply to the
> dark lwtheme as well, and there I think we do want the inverted icons?

Hmm, I thought I checked that case too, but appears you are right...  I'll try again.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 15 • 7 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=22cef8442b8254a847baa7396afb4ff593972952

	:Gijs (he/him) Reporter
	Comment 16 • 7 years ago
mozreview-review

Comment on attachment 8834538 [details]
Bug 1319034 - Disable tab close inversion with compact themes on Windows.

https://reviewboard.mozilla.org/r/110422/#review111736

::: browser/themes/windows/compacttheme.css:118
(Diff revision 2)
>  }
>  
> +/* Override tab close icon (to disable inversion) for better contrast with
> +   light theme on Windows 7 Classic theme. */
> +@media not all and (min-resolution: 1.1dppx) {
> +  #TabsToolbar[brighttext]:-moz-lwtheme-darktext .tab-close-button:not([selected="true"]) {

Nitty nit: I *think* this works if you put the pseudoselector on the .tab-close-button (before the :not(...) which would avoid layout having to check the descendant selector in the dark theme case.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 17 • 7 years ago
mozreview-review-reply

Comment on attachment 8834538 [details]
Bug 1319034 - Disable tab close inversion with compact themes on Windows.

https://reviewboard.mozilla.org/r/110422/#review111736

> Nitty nit: I *think* this works if you put the pseudoselector on the .tab-close-button (before the :not(...) which would avoid layout having to check the descendant selector in the dark theme case.

Yes, it should, I'll change it.

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 19 • 7 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c39622f261a9d39c0b467318343bd1eb385a0de3

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 20 • 7 years ago

Okay, looks good.  Tested configurations:

* Windows 7 basic theme with Compact Light theme (1dppx and 1.5dppx)
* Windows 7 basic theme with Compact Dark theme (1dppx and 1.5dppx)
* Windows 7 classic theme with Compact Light theme (1dppx and 1.5dppx)
* Windows 7 classic theme with Compact Dark theme (1dppx and 1.5dppx)
* Windows 10 with Compact Light theme (1dppx and 1.5dppx)
* Windows 10 with Compact Dark theme (1dppx and 1.5dppx)

	Pulsebot
	Comment 21 • 7 years ago

Pushed by jryans@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e872c7b798eb
Disable tab close inversion with compact themes on Windows. r=Gijs

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 22 • 7 years ago
bugherder

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

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow) Assignee
	Comment 23 • 7 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/8dcc2332e280ee23815b44ba76f78c9919d9ebd0