474655 - warn when nsIDOMCSSStyleDeclaration::GetPropertyCSSValue is called

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

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

[Alex Vincent [:WeirdAl]]

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

[Boris Zbarsky [:bzbarsky]]

> 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

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 358015 [details] [diff] [review]
patch

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

[Olli Pettay [:smaug][[e-mail]]]

Does the patch cause lots of warnings on some pages? Perhaps one warning per
document? (Similar to getBoxObjectFor warning)

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

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

[Olli Pettay [:smaug][[e-mail]]]

boxObjects are also gecko only.
For the boxObject warning there is a flag in nsDocument.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I backed this out because of bug 475311:

http://hg.mozilla.org/mozilla-central/rev/674246a64ed2
http://hg.mozilla.org/mozilla-central/rev/e2a26d16bf06

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

My current merged version of the patch (link stays current) is:
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/tip/warn-for-get-property-css-value
and today's version is:
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b8570a918f8c/warn-for-get-property-css-value

https://searchfox.org/mozilla-central/search?q=getPropertyCSSValue&case=true&path= is uses that we could fix...

Comment 10

[Boris Zbarsky [:bzbarsky]]

We should probably use a deprecated operation thing for this now that we have them.  That will give us telemetry in addition to the deprecation warning.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 474655: Add a deprecation warning + use counter to GetPropertyCSSValue.

The use counter comes built-in with the DeprecatedOperationList stuff.

Review commit: https://reviewboard.mozilla.org/r/227708/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/227708/

Comment 12

[Bobby Holley (:bholley)]

Comment on attachment 8958798 [details]
Bug 474655: Add a deprecation warning + use counter to GetPropertyCSSValue.

https://reviewboard.mozilla.org/r/227708/#review233570

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/151e6dc2df8e
Add a deprecation warning + use counter to GetPropertyCSSValue. r=bholley

Comment 14

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/151e6dc2df8e

Comment 15

[Kohei Yoshino]

Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/cssstyledeclaration-getpropertycssvalue-has-been-deprecated/