1291866 - replace inIDOMUtils.cssPropertyIsValid

Status: RESOLVED FIXED

(DevTools :: Framework, enhancement, P1)

Description

[Tom Tromey :tromey]

Bug 1069829 missed a few instances of cssPropertyIsValid.
See https://dxr.mozilla.org/mozilla-central/search?q=cssPropertyIsValid&redirect=true

Comment 1

[Greg Tatum [:gregtatum]]

Setting this to dependent on bug 1265796 as we're introducing a lot of async calls to the stack, and hopefully (maybe) once that lands it will make this bug a little more tractable.

Comment 2

[Greg Tatum [:gregtatum]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4282ec15c436

Comment 3

[Greg Tatum [:gregtatum]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=22e3e588e4bb

Comment 4

[Greg Tatum [:gregtatum]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3e087b755d2

Comment 5

[Greg Tatum [:gregtatum]]

Attachment: Bug 1291866 - Replace inIDOMUtils.cssPropertyIsValid;

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

Comment 7

[Tom Tromey :tromey]

Comment on attachment 8792520 [details]
Bug 1291866 - Replace inIDOMUtils.cssPropertyIsValid;

https://reviewboard.mozilla.org/r/79536/#review78198

::: devtools/client/shared/widgets/Tooltip.js:922
(Diff revision 1)
>   * CSSFilterEditorWidget.
>   *
>   * @param {Toolbox} toolbox
>   *        The devtools toolbox, needed to get the devtools main window.
>   */
> -function SwatchFilterTooltip(toolbox) {
> +function SwatchFilterTooltip(toolbox, cssIsValid) {

This needs an update to the function documentation.

::: devtools/shared/fronts/css-properties.js:101
(Diff revision 1)
> +      this._dummyElement = doc.createElement("div");
> +    }
> +
> +    // `!important` is not a valid value when setting a style declaration in the
> +    // CSS Object Model.
> +    const sanitizedValue = ("" + value).replace(/!important$\s*$/, "");

There can be spaces between the "!" and "important" tokens.

Comment 8

[Tom Tromey :tromey]

Comment on attachment 8792520 [details]
Bug 1291866 - Replace inIDOMUtils.cssPropertyIsValid;

https://reviewboard.mozilla.org/r/79536/#review78208

Thank you for doing this.

It looks good.  I had a couple of nits.

Comment 9

[Greg Tatum [:gregtatum]]

Comment on attachment 8792520 [details]
Bug 1291866 - Replace inIDOMUtils.cssPropertyIsValid;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79536/diff/1-2/

Comment 10

[Greg Tatum [:gregtatum]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=70a0519e68df

Comment 11

[Greg Tatum [:gregtatum]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8bbb81c95380

Comment 12

[Greg Tatum [:gregtatum]]

Comment on attachment 8792520 [details]
Bug 1291866 - Replace inIDOMUtils.cssPropertyIsValid;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79536/diff/2-3/

Comment 13

[Greg Tatum [:gregtatum]]

Comment on attachment 8792520 [details]
Bug 1291866 - Replace inIDOMUtils.cssPropertyIsValid;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79536/diff/3-4/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b65f0d1008a
Replace inIDOMUtils.cssPropertyIsValid; r=tromey

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4b65f0d1008a