622285 - CSS warnings: "Error in parsing value for 'size'/'position'. Declaration dropped." Solution: remove Rect_css method and instead allow iQ's css method to accept Rect arguments

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect, P2)

Description

[Michael Yoshitaka Erlewine [:mitcho]]

When Panorama is open, a lot of CSS warnings show up:

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

> Warning: Error in parsing value for 'position'.  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 that there are CSS declarations or dynamic styles which are not being applied as intended.

Comment 1

[Michael Yoshitaka Erlewine [:mitcho]]

So this has to do with the few instances where we give iQ's css() a Rect object. This is because right now we can't pass a rect into css(), but have to pass rect.css() into css(). This can be confusing and, as the warnings indicate, we have forgotten it in some places.

Solution: make it so iQ's css() *does* accept Rect objects directly. If the argument is a Rect, css() can turn it into the appropriate properties object.

This makes more sense than our current approach, as we ultimately want to move to using Geometry.jsm (bug 585672). The Rect class offered by Geometry.jsm has no .css(). As iQ is already aware of the Rect class, the proposed solution makes more sense and will alleviate a developer papercut.

Comment 2

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Patch v1

Comment 3

[Ian Gilman [:iangilman]]

Comment on attachment 500541 [details] [diff] [review]
Patch v1

Looks good, but to be complete you need to do the same for iQ's .animate.

R+ with that.

Comment 4

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Patch v1.1

Thanks Ian. Added the same treatment to animate.

Can we land this without a mochitest?

Comment 5

[Michael Yoshitaka Erlewine [:mitcho]]

Pushed patch v1.1 to try today. Passed.

Comment 6

[Ian Gilman [:iangilman]]

Yeah, I don't think it needs a test.

Comment 7

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

Comment on attachment 501166 [details] [diff] [review]
Patch v1.1

a=beltzner

Comment 8

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Patch for checkin

Comment 9

[Ian Gilman [:iangilman]]

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