Last Comment Bug 474655 - warn when nsIDOMCSSStyleDeclaration::GetPropertyCSSValue is called
: warn when nsIDOMCSSStyleDeclaration::GetPropertyCSSValue is called
Product: Core
Classification: Components
Component: DOM: CSS Object Model (show other bugs)
: Trunk
: All All
P3 normal with 1 vote (vote)
: ---
Assigned To: David Baron :dbaron: ⌚️UTC-8
: Jet Villegas (:jet)
Depends on: 475311
  Show dependency treegraph
Reported: 2009-01-21 11:44 PST by David Baron :dbaron: ⌚️UTC-8
Modified: 2010-10-13 19:35 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (8.27 KB, patch)
2009-01-21 12:59 PST, David Baron :dbaron: ⌚️UTC-8
bzbarsky: review+
bzbarsky: superreview+
Details | Diff | Splinter Review

Description User image David Baron :dbaron: ⌚️UTC-8 2009-01-21 11:44:13 PST
I think we should have a console warning when any of the implemementations of nsIDOMCSSStyleDeclaration::GetPropertyCSSValue are called, since this is an API that we're considering eliminating (I think we're the only implementor, and we implement it only for computed style, at considerable cost in code size).

So, for a start, I think we should add a warning to the error console that a deprecated API is being used, and then see if anybody complains...

Does that seem reasonable?
Comment 1 User image David Baron :dbaron: ⌚️UTC-8 2009-01-21 12:59:34 PST
Created attachment 358015 [details] [diff] [review]

I checked that the warning shows up for the computed style and style attribute cases, and that it doesn't in those cases for getPropertyValue.
Comment 2 User image Alex Vincent [:WeirdAl] 2009-01-21 13:21:27 PST
I wonder how we might write a test for this.  I know in xpcshell land, we can easily register a JS test component (see do_load_module in tools/test-harness/xpcshell-simple/head.js ), and that console listener components can register themselves with the console service.  But getComputedStyle requires a window (nsIDOMViewCSS), and I'm not sure if the style property of elements does.

So it might require a mochitest to test this, and I don't know if mochitests can register new components.
Comment 3 User image Boris Zbarsky [:bz] (still a bit busy) 2009-01-21 18:01:53 PST
> I wonder how we might write a test for this.

Mochitest.  You don't need a "js component" to register a console listener.  Heck, there are existing mochitests that do it that you should be able to crib from.
Comment 4 User image Boris Zbarsky [:bz] (still a bit busy) 2009-01-21 18:03:56 PST
Comment on attachment 358015 [details] [diff] [review]

This looks ok.  Some people actually use this API to get things like sizes in em, but ideally we'd implement a better CSSOM API they could use before this goes away completely.
Comment 5 User image Olli Pettay [:smaug] 2009-01-22 01:31:23 PST
Does the patch cause lots of warnings on some pages? Perhaps one warning per
document? (Similar to getBoxObjectFor warning)
Comment 6 User image David Baron :dbaron: ⌚️UTC-8 2009-01-22 20:30:17 PST

Where does the getBoxObjectFor warning store the state that it's given the warning already?  In the property table or something?

(My understanding is that we're the only browser implementing this API, so I'm not expecting to see a lot of users in the real world.)
Comment 7 User image Olli Pettay [:smaug] 2009-01-23 02:43:50 PST
boxObjects are also gecko only.
For the boxObject warning there is a flag in nsDocument.
Comment 8 User image David Baron :dbaron: ⌚️UTC-8 2009-02-04 13:45:31 PST
I backed this out because of bug 475311:

Note You need to log in before you can comment on or make changes to this bug.