1255401 - inIDOMUtils.getCSSValuesForProperty() is missing keyword for 'quotes'

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Sebastian Zartner [:sebo]]

The 'quotes' property is missing its keyword 'none'.

Test case (to execute in Scratchpad):

let DOMUtils = Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils);

DOMUtils.getCSSValuesForProperty("quotes");

Sebastian

Comment 1

[Tom Tromey :tromey]

Attachment: Bug 1255401 - fix values allowed for "all" property;

Re-running mach devtools-css-db showed that the database was slightly
out of date relative to the tree.  I broke this out to a separate
patch to make it less obscure.

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

Comment 2

[Tom Tromey :tromey]

Attachment: Bug 1255401 - fix getCSSValuesForProperty result for "quotes" property;

It seemed valid to simply set the expected variants in
nsCSSPropList.h; other properties (e.g., align-content) do this as
well.

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

Comment 3

[Cameron McCormack (:heycam)]

Comment on attachment 8941599 [details]
Bug 1255401 - fix getCSSValuesForProperty result for "quotes" property;

https://reviewboard.mozilla.org/r/211860/#review217696

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 8941598 [details]
Bug 1255401 - fix values allowed for "all" property;

https://reviewboard.mozilla.org/r/211858/#review217698

This grayscale keyword seems to pop in and out of existence and I'm not sure why.

But anyway, I really wonder if we should be generating this big list of values for the all shorthand, since in reality the only values you can use are the CSS-wide keywords.  Instead of doing this update, could you look at how we generate this, and make sure we only output inherit / initial / unset?

Comment 5

[Tom Tromey :tromey]

(In reply to Cameron McCormack (:heycam) from comment #4)

> This grayscale keyword seems to pop in and out of existence and I'm not sure
> why.

Maybe prefs or build options affect it somehow.

> But anyway, I really wonder if we should be generating this big list of
> values for the all shorthand, since in reality the only values you can use
> are the CSS-wide keywords.  Instead of doing this update, could you look at
> how we generate this, and make sure we only output inherit / initial / unset?

Thanks for noticing this.

It's probably because `gAllSubpropTable` lists everything.
I think special-casing "all" in `InspectorUtils::GetCSSValuesForProperty` is probably
the correct approach.

Also, "all" takes "revert" as well (according to https://developer.mozilla.org/en-US/docs/Web/CSS/all),
but I don't see that in nsCSSParser.cpp.

Comment 6

[Tom Tromey :tromey]

Comment on attachment 8941598 [details]
Bug 1255401 - fix values allowed for "all" property;

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

Comment 7

[Tom Tromey :tromey]

Comment on attachment 8941599 [details]
Bug 1255401 - fix getCSSValuesForProperty result for "quotes" property;

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

Comment 8

[Cameron McCormack (:heycam)]

(In reply to Tom Tromey :tromey from comment #5)
> Also, "all" takes "revert" as well (according to
> https://developer.mozilla.org/en-US/docs/Web/CSS/all),
> but I don't see that in nsCSSParser.cpp.

Yes, we don't implement revert yet.

Comment 9

[Cameron McCormack (:heycam)]

Comment on attachment 8941598 [details]
Bug 1255401 - fix values allowed for "all" property;

https://reviewboard.mozilla.org/r/211858/#review218022

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52fbf0d6c18c
fix values allowed for "all" property; r=heycam
https://hg.mozilla.org/integration/autoland/rev/1572cddc9e46
fix getCSSValuesForProperty result for "quotes" property; r=heycam

Comment 11

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/52fbf0d6c18c
https://hg.mozilla.org/mozilla-central/rev/1572cddc9e46

Comment 12

[Sebastian Zartner [:sebo]]

Works fine for me on Nightly 2018-01-14. Thank you for the patch, Tom!

Sebastian