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

VERIFIED FIXED in Firefox 59

Status

()

VERIFIED FIXED
3 years ago
9 months ago

People

(Reporter: sebo, Assigned: tromey)

Tracking

(Blocks: 1 bug)

Trunk
mozilla59
Points:
---

Firefox Tracking Flags

(firefox59 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
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
(Assignee)

Updated

a year ago
Assignee: nobody → ttromey
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
Comment on attachment 8941599 [details]
Bug 1255401 - fix getCSSValuesForProperty result for "quotes" property;

https://reviewboard.mozilla.org/r/211860/#review217696
Attachment #8941599 - Flags: review?(cam) → review+

Comment 4

a year ago
mozreview-review
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?
(Assignee)

Comment 5

a year ago
(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 hidden (mozreview-request)
Comment hidden (mozreview-request)
(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

a year ago
mozreview-review
Comment on attachment 8941598 [details]
Bug 1255401 - fix values allowed for "all" property;

https://reviewboard.mozilla.org/r/211858/#review218022
Attachment #8941598 - Flags: review?(cam) → review+

Comment 10

a year ago
Pushed by ttromey@mozilla.com:
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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/52fbf0d6c18c
https://hg.mozilla.org/mozilla-central/rev/1572cddc9e46
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
(Reporter)

Comment 12

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

Sebastian
Status: RESOLVED → VERIFIED
status-firefox59: fixed → verified
status-firefox48: affected → ---
You need to log in before you can comment on or make changes to this bug.