Closed Bug 1643230 Opened 1 month ago Closed 1 month ago

CSS parser isn't reporting first error after good declaration

Categories

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

Unspecified
All
defect

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox-esr68 --- wontfix
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- wontfix
firefox79 --- fixed

People

(Reporter: tustamido, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

Given the following CSS code as example:
* { color: blue; abc; def; ghi; jkl; }

CSS parser should report 4 errors, as it happens when using W3C Validator: https://jigsaw.w3.org/css-validator/

Instead, Firefox reports only 3, it skips "abc;".

Code to reproduce (just paste it on Browser Console):

docShell.cssErrorReportingEnabled = true;
let errorListener = {
  observe: function (message) {
    let error = message.QueryInterface(Ci.nsIScriptError);
    console.log(error.errorMessage);
  }
};
Services.console.registerListener(errorListener);
let styleEl = document.createElement('style');
styleEl.appendChild(document.createTextNode('* { color: blue; abc; def; ghi; jkl; }'));
document.documentElement.appendChild(styleEl);

Using mozregression, mozilla-central 2019-05-08 was good (reports all 4 erros, including "abc;"), 2019-05-09 was bad. But I can't go deeper than this as it seems inbound/autoland builds are deleted after 365 days (am I right in this?).

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fd9b8188501938a9306105a01db5beeefeab1a19&tochange=2abcefb31ba7b2a1c573edc5695b772826c6a078

I think I can make an educated guess which of those caused the regression, though I didn't test.

Regressed by: 1538101

Yeah, I can probably take it anyhow. Thanks David :)

Assignee: nobody → emilio
Severity: -- → S3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(emilio)
Priority: -- → P3
Flags: needinfo?(emilio)

Rather than waiting until parsing another id (successfully or
unsuccessfully).

If we error before we even get to PropertyId::parse, we'd incorrectly
associate the error with the wrong property, incorrectly omitting it
sometimes.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/08748e5b0fd3
Clear last_parsed_property_id right after successfully parsing the value. r=jwatt
Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(emilio)

Not really, long-standing issue.

Flags: needinfo?(emilio)
You need to log in before you can comment on or make changes to this bug.