821969 - Un-prefix gradients from per-theme browser.css

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Soumya Deb [:Debloper]]

Remove stray traces of -moz in browser.css from WinStripe|PinStripe|GnomeStripe themes.

Comment 1

[Soumya Deb [:Debloper]]

Attachment: WIP Patch

-moz-linear-gradient(bottom left, <colors>) !== linear-gradient(to top right, <colors>)

Flagged the occurrences with comments - not sure how to deal with it.

http://dev.w3.org/csswg/css3-images/#linear-gradients shows the change in spec, how the standard implementation handles the gradient line differently with keyword-angle notation.

Suggestion?

Comment 2

[Masatoshi Kimura [:emk]]

Comment on attachment 692745 [details] [diff] [review]
WIP Patch

>-  background-image: -moz-repeating-linear-gradient(-45deg,
>+  background-image: repeating-linear-gradient(-45deg,

This is wrong. <unprefixed-degree> = 90deg - <prefixed-degree>. (135deg in this case.) Same for all occurrence of -45deg.

>@@ -2323,17 +2323,17 @@ html|*#gcli-output-frame {
>-  background-image: -moz-linear-gradient(top, #B4211B, #8A1915);
>+  background-image: linear-gradient(to top, #8A1915, #B4211B);

Why don't you just writing
  background-image: linear-gradient(#B4211B, #8A1915);
?

>@@ -3780,17 +3780,17 @@ html|*#gcli-output-frame {
>-  background-image: -moz-linear-gradient(top, #B4211B, #8A1915);
>+  background-image: linear-gradient(to top, #8A1915, #B4211B);

Same here.

Comment 3

[Masatoshi Kimura [:emk]]

(In reply to Soumya Deb [:Debloper] from comment #1)
> -moz-linear-gradient(bottom left, <colors>) !== linear-gradient(to top
> right, <colors>)
> 
> Flagged the occurrences with comments - not sure how to deal with it.
> 
> http://dev.w3.org/csswg/css3-images/#linear-gradients shows the change in
> spec, how the standard implementation handles the gradient line differently
> with keyword-angle notation.
> 
> Suggestion?

Did you *really* want the old behavior?

Comment 4

[Soumya Deb [:Debloper]]

(In reply to Masatoshi Kimura [:emk] from comment #2)
> Comment on attachment 692745 [details] [diff] [review]
> <unprefixed-degree> = 90deg - <prefixed-degree>.

Right, will fix.


> >+  background-image: linear-gradient(to top, #8A1915, #B4211B);
> 
> Why don't you just writing
>   background-image: linear-gradient(#B4211B, #8A1915);

Makes sense, will fix (actually started doing, then wasn't sure if I should, so reverted).


(In reply to Masatoshi Kimura [:emk] from comment #3)
> (In reply to Soumya Deb [:Debloper] from comment #1)
> Did you *really* want the old behavior?

I don't wanna change the behavior of the code, just standardize them with proper syntax. Altering gradient behaviour etc. might require extra UI-review and isn't the primary purpose of the bug. So preferably, yes.

Comment 5

[Masatoshi Kimura [:emk]]

(In reply to Soumya Deb [:Debloper] from comment #4)
> > Did you *really* want the old behavior?
> 
> I don't wanna change the behavior of the code, just standardize them with
> proper syntax. Altering gradient behaviour etc. might require extra
> UI-review and isn't the primary purpose of the bug. So preferably, yes.

New syntax does not provide a way to make an exact copy of old behavior. Perhaps you will have to file a followup bug for the behavior change.

Comment 6

[Soumya Deb [:Debloper]]

Attachment: Patch for all themes

Comment 7

[Soumya Deb [:Debloper]]

(In reply to Masatoshi Kimura [:emk] from comment #5)
> (In reply to Soumya Deb [:Debloper] from comment #4)
> > > Did you *really* want the old behavior?
> > 
> > ...So preferably, yes.
> 
> Perhaps you will have to file a followup bug for the behavior change.

Done: #822322

Intentionally kept "to left", "to right" as it is, cause changing to angular notation makes it less legible. All vertical gradients are formatted to be "to bottom" & the direction keyword removed (for being default).

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 692985 [details] [diff] [review]
Patch for all themes

> #urlbar[pageproxystate="valid"] > #identity-box.verifiedIdentity {
>   background-color: #fff;
>   color: hsl(92,100%,30%);
>   -moz-margin-end: 4px;
>-  background-image: -moz-linear-gradient(hsla(92,81%,16%,0),
>+  background-image: linear-gradient(hsla(92,81%,16%,0),
>                                          hsla(92,81%,16%,.2) 25%,
>                                          hsla(92,81%,16%,.2) 75%,
>                                          hsla(92,81%,16%,0));

adjust indentation

> #TabsToolbar[tabsontop=false] {
>   background-image:
>-    -moz-linear-gradient(bottom, rgba(0,0,0,.3) 1px, rgba(0,0,0,.05) 1px, transparent 50%);
>+    linear-gradient(transparent 50%, rgba(0,0,0,.05) 1px, rgba(0,0,0,.3) 1px);

This results in a different gradient. (You can't go from 50% to 1px, since 50% is larger than 1px here.)

Comment 9

[Soumya Deb [:Debloper]]

Attachment: Patch for all themes

Reverted all the vertical gradients using multi-unit, or weighted-sized values to as was in https://bugzilla.mozilla.org/attachment.cgi?id=692745&action=diff

Indentations fixed on multi-lined gradient declarations. Hadn't done so before to minimize number of lines changed.

Comment 10

[Soumya Deb [:Debloper]]

Also, not sure if I've done right with: https://bugzilla.mozilla.org/attachment.cgi?id=696619&action=diff#a/browser/themes/winstripe/browser.css_sec10 line #825

Making it `linear-gradient(to top, transparent, rgba(0,0,0,.15))` would surely look more homogeneous with the other lines. Please tell if I should change to that instead.

Comment 11

[Dão Gottwald [:dao]]

(In reply to Soumya Deb [:Debloper] from comment #10)
> Also, not sure if I've done right with:
> https://bugzilla.mozilla.org/attachment.cgi?id=696619&action=diff#a/browser/
> themes/winstripe/browser.css_sec10 line #825
> 
> Making it `linear-gradient(to top, transparent, rgba(0,0,0,.15))` would
> surely look more homogeneous with the other lines. Please tell if I should
> change to that instead.

Yep.

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 696619 [details] [diff] [review]
Patch for all themes

> #addon-bar[customizing] > #status-bar {
>   opacity: .5;
>-  background-image: -moz-repeating-linear-gradient(-45deg,
>-                                                   rgba(255,255,255,.3), rgba(255,255,255,.3) 5px,
>-                                                   rgba(0,0,0,.3) 5px, rgba(0,0,0,.3) 10px);
>+  background-image: repeating-linear-gradient(-45deg,
>+                                              rgba(255,255,255,.3), rgba(255,255,255,.3) 5px,
>+                                              rgba(0,0,0,.3) 5px, rgba(0,0,0,.3) 10px);
> }

Don't you need to update the angle here?

Comment 13

[Soumya Deb [:Debloper]]

Attachment: Patch for all themes

(In reply to Dão Gottwald [:dao] from comment #12)
> Comment on attachment 696619 [details] [diff] [review]
> ...
> >+  background-image: repeating-linear-gradient(-45deg,
> ...
> Don't you need to update the angle here?

*Facepalm* and at loss of excuses!

Comment 14

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/04557efa1fd9

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/04557efa1fd9