Closed Bug 1379052 Opened 5 years ago Closed 4 years ago

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

Categories

(Firefox :: Theme, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 57
Iteration:
57.1 - Aug 15
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- disabled
firefox57 --- verified

People

(Reporter: Dolske, Assigned: dao)

References

Details

(Keywords: regression, Whiteboard: [photon-visual][p3])

Attachments

(2 files, 1 obsolete file)

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
Keywords: regression
Whiteboard: [photon-visual][triage]
Attached patch Patch v.1 (obsolete) — Splinter Review
Assignee: nobody → dolske
Attachment #8884577 - Flags: review?(dao+bmo)
Status: NEW → ASSIGNED
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-visual][triage] → [photon-visual]
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.
Attachment #8884577 - Flags: review?(dao+bmo) → review-
Whiteboard: [photon-visual] → [photon-visual][p3]
Flags: qe-verify? → qe-verify+
QA Contact: brindusa.tot
Attachment #8884577 - Attachment is obsolete: true
Assignee: dolske → dao+bmo
Iteration: --- → 57.1 - Aug 15
Blocks: 1383312
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
Attachment #8893363 - Flags: review?(dharvey) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e4a76e0d1fe7
Fix [attention] styling in unpinned tabs. r=daleharvey
https://hg.mozilla.org/mozilla-central/rev/e4a76e0d1fe7
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
No longer blocks: 1383449
Duplicate of this bug: 1383449
QA Contact: brindusa.tot → ovidiu.boca
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.