Un-prefix gradients from per-theme browser.css

RESOLVED FIXED in Firefox 20

Status

()

Firefox
Theme
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Debloper, Assigned: Debloper)

Tracking

Trunk
Firefox 20
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

5 years ago
Remove stray traces of -moz in browser.css from WinStripe|PinStripe|GnomeStripe themes.
(Assignee)

Updated

5 years ago
Assignee: nobody → debloper
Blocks: 818659
(Assignee)

Updated

5 years ago
Component: General → Theme
(Assignee)

Comment 1

5 years ago
Created attachment 692745 [details] [diff] [review]
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?
Attachment #692745 - Flags: review?(dao)
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.
(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?
(Assignee)

Comment 4

5 years ago
(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.
Status: NEW → ASSIGNED
(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.
(Assignee)

Comment 6

5 years ago
Created attachment 692985 [details] [diff] [review]
Patch for all themes
Attachment #692745 - Attachment is obsolete: true
Attachment #692745 - Flags: review?(dao)
Attachment #692985 - Flags: review?(dao)
(Assignee)

Comment 7

5 years ago
(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).
See Also: → bug 822322
Target Milestone: --- → Firefox 20
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.)
Attachment #692985 - Flags: review?(dao) → review-
(Assignee)

Comment 9

5 years ago
Created attachment 696619 [details] [diff] [review]
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.
Attachment #692985 - Attachment is obsolete: true
Attachment #696619 - Flags: review?(dao)
(Assignee)

Comment 10

5 years ago
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.
(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 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?
(Assignee)

Comment 13

4 years ago
Created attachment 696750 [details] [diff] [review]
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!
Attachment #696619 - Attachment is obsolete: true
Attachment #696619 - Flags: review?(dao)
Attachment #696750 - Flags: review?(dao)

Updated

4 years ago
Attachment #696750 - Flags: review?(dao) → review+

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/04557efa1fd9
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/04557efa1fd9
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.