1192493 - Move tab-icon-overlay and tab-icon-sound display rules from theme to content stylesheet

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Dão Gottwald [:dao]]

Attachment: patch

Comment 1

[(no longer active)]

Comment on attachment 8645312 [details] [diff] [review]
patch

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

If you'd like to keep the hunks that remove [pinned], please rebase on top of bug 1192568.

::: browser/themes/shared/tabs.inc.css
@@ -112,4 @@
>    border-radius: 8px;
>  }
>  
> -.tab-icon-overlay[soundplaying][pinned]:hover,

Why are you removing the [pinned] attributes here?  I don't understand why these changes are needed in this patch.

Comment 2

[Dão Gottwald [:dao]]

(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #1)
> > -.tab-icon-overlay[soundplaying][pinned]:hover,
> 
> Why are you removing the [pinned] attributes here?

Because they're useless.

Comment 3

[(no longer active)]

Comment on attachment 8645312 [details] [diff] [review]
patch

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

r=me on a rebased version on top of bug 1192568.

Note that I am landing bug 1192568 on inbound today when it reopens, and landing this patch unmodified (especially on fx-team) will create merge issues for the sheriffs.

Comment 4

[Matthew N. [:MattN]]

(In reply to (Ask others for review please; out most of the week) from comment #3)
> Note that I am landing bug 1192568 on inbound today when it reopens, and
> landing this patch unmodified (especially on fx-team) will create merge
> issues for the sheriffs.

Please use fx-team for front-end related changes. The point of separating the branches by functional area is to avoid such merge issues.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/5ac45627b8bd

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5ac45627b8bd

Comment 7

[Alfred Kayser]

Will this be also uplifted to 42, so that full themes won't have to include these "display:" rules for compatibility with FF?

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8645312 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 486262
[User impact if declined]: addon compat, see comment 7
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: low, mostly moving code
[String/UUID change made/needed]: none

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8645312 [details] [diff] [review]
patch

Let's do it to not upset our addon developers!

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ae9967e511b3