622288 - CSS warnings: "Error in parsing value for 'height'/'width'. Declaration dropped."

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect, P4)

Description

[Michael Yoshitaka Erlewine [:mitcho]]

From time to time in Panorama, we get CSS warnings like this:

> Warning: Error in parsing value for 'height'.  Declaration dropped.
> Source File: chrome://browser/content/tabview.html
> Line: 0

> Warning: Error in parsing value for 'width'.  Declaration dropped.
> Source File: chrome://browser/content/tabview.html
> Line: 0

We need to clean these up. They're messy and unprofessional, and may actually
indicate underlying problems.

See also: bug 622285

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

Just in case to save some time:

-      height: orig.height * zoomWidth / orig.width
+      height: orig.height * zoomWidth / orig.width || 0
--> sometimes orig.width == 0, so divide by zero and get NaN, fallback to 0

-  background-image-opacity: .1;
--> this property does not exist

-  -moz-padding-end: -20px;
--> this property does not accept negative values

Comment 2

[Ian Gilman [:iangilman]]

Comment on attachment 503985 [details] [diff] [review]
patch v1

>-      height: orig.height * zoomWidth / orig.width
>+      height: orig.height * zoomWidth / orig.width || 0

Might be worth a comment.

>diff --git a/browser/themes/gnomestripe/browser/tabview/tabview.css b/browser/themes/gnomestripe/browser/tabview/tabview.css
>--- a/browser/themes/gnomestripe/browser/tabview/tabview.css
>+++ b/browser/themes/gnomestripe/browser/tabview/tabview.css
>@@ -429,10 +429,8 @@
>   font-style: italic !important;
>   color: transparent;
>   background-image: url(chrome://browser/skin/tabview/edit-light.png);
>-  background-image-opacity: .1;
>   background-repeat: no-repeat;
>   -moz-padding-start: 20px;
>-  -moz-padding-end: -20px;
> }
> 
> .title-container:hover input.name:-moz-placeholder {
>diff --git a/browser/themes/pinstripe/browser/tabview/tabview.css b/browser/themes/pinstripe/browser/tabview/tabview.css
>--- a/browser/themes/pinstripe/browser/tabview/tabview.css
>+++ b/browser/themes/pinstripe/browser/tabview/tabview.css
>@@ -421,7 +421,6 @@
>   font-style: italic !important;
>   color: transparent;	
>   background-image: url(chrome://browser/skin/tabview/edit-light.png);
>-  background-image-opacity: .1;
>   background-repeat: no-repeat;
>   -moz-padding-start: 20px;
>   -moz-margin-end: -20px;
>diff --git a/browser/themes/winstripe/browser/tabview/tabview.css b/browser/themes/winstripe/browser/tabview/tabview.css
>--- a/browser/themes/winstripe/browser/tabview/tabview.css
>+++ b/browser/themes/winstripe/browser/tabview/tabview.css
>@@ -448,10 +448,8 @@
>   font-style: italic !important;
>   color: transparent;
>   background-image: url(chrome://browser/skin/tabview/edit-light.png);
>-  background-image-opacity: .1;
>   background-repeat: no-repeat;
>   -moz-padding-start: 20px;
>-  -moz-padding-end: -20px;
> }

Good clean up, and doesn't seem to have any negative impact.

I wonder if the -moz-margin-end in pinstripe... maybe we should get rid of that as well? Or convert the -moz-padding-ends to margin?

Comment 3

[Dão Gottwald [:dao]]

(In reply to comment #2)
> Comment on attachment 503985 [details] [diff] [review]
> patch v1
> 
> >-      height: orig.height * zoomWidth / orig.width
> >+      height: orig.height * zoomWidth / orig.width || 0
> 
> Might be worth a comment.

Or write it so that it's self-explanatory.
E.g. height: (orig.width ? orig.height * zoomWidth / orig.width : 0)

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

(In reply to comment #3)
> Or write it so that it's self-explanatory.
> E.g. height: (orig.width ? orig.height * zoomWidth / orig.width : 0)

Looks good, I'll take that.

(In reply to comment #2)
> I wonder if the -moz-margin-end in pinstripe... maybe we should get rid of that
> as well? Or convert the -moz-padding-ends to margin?

Well, negative margin values are allowed and _could_ make sense. I don't know if they're really needed for the layout because gnomestripe doesn't use them :/

Comment 5

[Ian Gilman [:iangilman]]

Comment on attachment 503985 [details] [diff] [review]
patch v1

(In reply to comment #4)
> (In reply to comment #3)
> > Or write it so that it's self-explanatory.
> > E.g. height: (orig.width ? orig.height * zoomWidth / orig.width : 0)
> 
> Looks good, I'll take that.
> 
> (In reply to comment #2)
> > I wonder if the -moz-margin-end in pinstripe... maybe we should get rid of that
> > as well? Or convert the -moz-padding-ends to margin?
> 
> Well, negative margin values are allowed and _could_ make sense. I don't know
> if they're really needed for the layout because gnomestripe doesn't use them :/

I've just tried it on my mac; doesn't seem to make any difference. Please kill it.

R+ with those two changes.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v2

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 505208 [details] [diff] [review]
patch v2

a=beltzner

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch for checkin

Comment 9

[Ian Gilman [:iangilman]]

http://hg.mozilla.org/mozilla-central/rev/f68e1a3a3705

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Backed out: http://hg.mozilla.org/mozilla-central/rev/84431e829386

This one was backed out together with bug 625424 but surely is not the cause for tests failing. So please feel free to push again :)

Comment 11

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/bb6aa94958c4

Comment 12

[Oscar Quinteros]

I been getting this on a single page app using React and I do not get this on Google Chrome latest but application seen to continue to work correctly using Firefox 89.0 (64-bit)

Error in parsing value for ‘direction’. Declaration dropped.

Comment 13

[Oscar Quinteros]

Error in parsing value for ‘direction’. Declaration dropped
When I clicked on the error file link takes me to ReactCrop.css