1379052 - Big blue circles behind favicon on tabs opened via Send To Device

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Justin Dolske [:Dolske]]

Attachment: Image 2017-07-06 at 7.41.39 PM.png

The new tab indicator from bug 1375893 is being shown as a big blue circle behind the favicon on tabs that are opened via Send To Device. (See screenshot)

Reported by shorlander, and I can reproduce.

These rules seem to be applied:

http://searchfox.org/mozilla-central/rev/f1472e5b57fea8d4a7bbdd81c44e46958fe3c1ce/browser/themes/shared/tabs.inc.css#485-488

Disabling the background-size fixes the sizing of it, but the icon is then at the bottom-left of the favicon. "background-position: left 11px bottom -3px;"

The "11px" value seems a little magic, alas. I guess that's the 9px padding from http://searchfox.org/mozilla-central/rev/f1472e5b57fea8d4a7bbdd81c44e46958fe3c1ce/browser/themes/shared/tabs.inc.css#65 (which gets the indicator left-aligned with the favicon), plus 2px to get it centered? [Yeah, the dot is 12px wide, so 12+2+2 = std. 16px favicon.]

Presumably need to fix these for compact themes too: http://searchfox.org/mozilla-central/rev/f1472e5b57fea8d4a7bbdd81c44e46958fe3c1ce/browser/themes/shared/compacttheme.inc.css#340-343

Comment 1

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8884577 [details] [diff] [review]
Patch v.1

>--- a/browser/themes/shared/tabs.inc.css
>+++ b/browser/themes/shared/tabs.inc.css
>@@ -482,9 +482,14 @@
>   background-repeat: no-repeat;
> }
> 
>+/* The titlechanged indicator is also used when a tab is opened via Send To Device. */

This comment is rather confusing because "titlechanged indicator" lacks context. Note that the rule uses an attribute called attention, not titlechanged.

> .tabbrowser-tab[image] > .tab-stack > .tab-content[attention]:not([pinned]):not([selected="true"]) {
>+%ifdef MOZ_PHOTON_THEME
>+  background-position: left 11px bottom -3px;
>+%else
>   background-position: left bottom var(--tab-toolbar-navbar-overlap);
>   background-size: 34px 100%;
>+%endif

I think var(--tab-toolbar-navbar-overlap) should play into the -3px thingy, and then you can probably get rid of the compacttheme.inc.css rule for photon. --tab-toolbar-navbar-overlap is 1px by default, but set to 0 in compacttheme.inc.css and on Windows 10.

Comment 3

[Dão Gottwald [:dao]]

Attachment: Bug 1379052 - Fix [attention] styling in unpinned tabs.

Review commit: https://reviewboard.mozilla.org/r/164458/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164458/

Comment 4

[Dale Harvey (:daleharvey)]

Comment on attachment 8893363 [details]
Bug 1379052 - Fix [attention] styling in unpinned tabs.

https://reviewboard.mozilla.org/r/164458/#review170570

Sorry it took me a while to reproduce this but see the fix now, looks good cheers

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e4a76e0d1fe7
Fix [attention] styling in unpinned tabs. r=daleharvey

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[ovidiu boca[:Ovidiu]]

I verified this issue on Mac OS X 10.10, Windows 7 and 10, Ubuntu 16.04 with Nightly 57.0a1(2017-08-21) and I can't reproduce this issue, I will mark this as verified fixed.