155855 - turn CSS error reporting on for DEBUG builds

Status: RESOLVED FIXED

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

Description

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

I'm tired of seeing new CSS errors creep into the chrome.  I expect authors of
chrome CSS are also annoyed at the lack of error reporting.   So I want to turn
on CSS error reporting in DEBUG builds.

I'll attach a patch to do this, and also to enhance the reporting a bit.

Comment 1

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

Attachment: CSS error reporting in DEBUG builds

Comment 2

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

For reference, see bug 139942 and bug 65469.  In particular, bug 139942 will
shortly have a patch that fixes all the CSS errors in the tree, although I don't
intend to check it in.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> +      REPORT_UNEXPECTED_EOF(NS_LITERAL_STRING("end of @media media list"));

This could happen for a media list attached to an @import rule as well.

> +    REPORT_UNEXPECTED_EOF(NS_LITERAL_STRING("end of @import rule"));

This should be something like "URI to import in @import rule"

> +    REPORT_UNEXPECTED_EOF(NS_LITERAL_STRING("namespace prefix"));

"namespace prefix in @namespace rule" ?

> +      REPORT_UNEXPECTED_EOF(NS_LITERAL_STRING("namespace URI"));

"namespace URI in @namespace rule" ?

> +      REPORT_UNEXPECTED_EOF(NS_LITERAL_STRING(", or {"));

Maybe put the ',' and '{' in single quotes?

> +        REPORT_UNEXPECTED_EOF(NS_LITERAL_STRING("; or } to end declaration"));

Same here

> +      REPORT_UNEXPECTED_EOF(NS_LITERAL_STRING("; or } to end declaration"));

And here

Other things in nsCSSParser:

Could you change 

394 #define REPORT_UNEXPECTED_TOKEN(_reas) \
395   ReportUnexpectedToken(mScanner,mToken,_reas)

to have "_reason" instead?  There's no reason to lose those extra few chars and
it makes things more readable, imo.

> +  mScanner->AddToError(NS_LITERAL_STRING("Unexpected end of file while
searching for ") + _err)

Maybe append a '.' on the end, a la ReportUnexpectedToken()?

Things in nsCSSScanner:

252   printf("CSS Error (%s :%u.%u): %s\n",

fprintf(stderr, ... ) please?  Or do we really want this going to stdout?

Comment 4

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

Attachment: revised patch

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 90277 [details] [diff] [review]
revised patch

r=bzbarsky

Comment 6

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 90277 [details] [diff] [review]
revised patch

sr=[e-mail]

Comment 7

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

Fix checked in 2002-07-08 21:28 PDT.

Comment 8

[leonard]

xref: 162425