1211330 - Remove CSS pref "layout.css.unset-value.enabled"

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Daniel Holbert [:dholbert]]

As heycam noted parenthetically in bug 1210905 comment 19:
> >  if (IsCSSPropertyPrefEnabled("layout.css.unset-value.enabled")) {
>
> (At this point it's probably safe to remove this pref...)

I take it this feature is mature/baked enough that there's no point in having it be pref-toggleable anymore.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I suspect this pref doesn't work at all now, I don't remember any code handling it on stylo.

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1211330 - Removed CSS pref layout.css.unset-value.enabled -

Feature is mature and doesn't need pref.
In fact, it was not used in code anymore, only in tests.

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

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 8988391 [details]
Bug 1211330 - Removed CSS pref layout.css.unset-value.enabled -

https://reviewboard.mozilla.org/r/253670/#review260588

::: layout/style/test/property_database.js:2765
(Diff revision 1)
>      "none, none, none, none, none",
>      "url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAAKElEQVR42u3NQQ0AAAgEoNP+nTWFDzcoQE1udQQCgUAgEAgEAsGTYAGjxAE/G/Q2tQAAAABJRU5ErkJggg==), none",
>      "none, url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAAKElEQVR42u3NQQ0AAAgEoNP+nTWFDzcoQE1udQQCgUAgEAgEAsGTYAGjxAE/G/Q2tQAAAABJRU5ErkJggg==), none",
>      "url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAAKElEQVR42u3NQQ0AAAgEoNP+nTWFDzcoQE1udQQCgUAgEAgEAsGTYAGjxAE/G/Q2tQAAAABJRU5ErkJggg==), url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAAKElEQVR42u3NQQ0AAAgEoNP+nTWFDzcoQE1udQQCgUAgEAgEAsGTYAGjxAE/G/Q2tQAAAABJRU5ErkJggg==)",
>      ].concat(validGradientAndElementValues),
> -    invalid_values: [
> +    invalid_values: [ "linear-gradient(unset, 10px 10px, from(blue))", "linear-gradient(unset, 10px 10px, blue 0)", "repeating-linear-gradient(unset, 10px 10px, blue 0)"

Let's put these in invalidGradientAndElementValues instead, so they can be reused in various properties that take CSS images.

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1211330 - Moved 'unset' invalid values from background-image alone to shared invalidGradientAndElementValues -

This expands the testing of 'unset' invalid values to border-image-source,
mask-image, and shape-outside.

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

Comment 6

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8988391 [details]
Bug 1211330 - Removed CSS pref layout.css.unset-value.enabled -

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

Comment 7

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8988391 [details]
Bug 1211330 - Removed CSS pref layout.css.unset-value.enabled -

https://reviewboard.mozilla.org/r/253670/#review260588

> Let's put these in invalidGradientAndElementValues instead, so they can be reused in various properties that take CSS images.

As discussed, I've added a separate patch, as it's not related to the pref and adds new tests.

Comment 8

[Cameron McCormack (:heycam)]

Comment on attachment 8989312 [details]
Bug 1211330 - Moved 'unset' invalid values from background-image alone to shared invalidGradientAndElementValues -

https://reviewboard.mozilla.org/r/254370/#review261190

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd647d6acb5a
Removed CSS pref layout.css.unset-value.enabled - r=heycam
https://hg.mozilla.org/integration/autoland/rev/7ff340d8288f
Moved 'unset' invalid values from background-image alone to shared invalidGradientAndElementValues - r=heycam

Comment 10

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/bd647d6acb5a
https://hg.mozilla.org/mozilla-central/rev/7ff340d8288f

Comment 11

[Estelle Weyl]

What dev docs need updating? There are no flags, quirks, or notes regarding unset in the browser compatibility charts. If I understand correctly, this fixed occurrences of unset making a css property fail when it shouldn't and not fail when it should fail.

Comment 12

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

Yeah, given that there's no mention of the pref in https://developer.mozilla.org/en-US/docs/Web/CSS/unset I don't see anything that needs to be done.

Comment 13

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(In reply to Estelle Weyl from comment #11)
> [...] If I understand correctly, this
> fixed occurrences of unset making a css property fail when it shouldn't and
> not fail when it should fail.

This bug here removed the now-unused about:config preference 'layout.css.unset-value.enabled', and also improved the test coverage.
The 'unset' feature itself was not touched in this bug.
So as David confirmed, no docs should need updating.