264162 - css error reporting should be pref-controlled

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Gervase Markham [:gerv]]

Currently, CSS error-reporting is a build-time option. Would there be
significant performance or bloat impact if it were made a pref or JS Console
toggle instead? 

Rationale: At least one person has complained to me that they don't like using
the suite for web development because JS errors get lost among the CSS errors of
other sites they are visiting. Also, it would then more closely match e.g.
dump() support and JS Strict warnings support, which are both prefs.

Gerv

Comment 1

[Boris Zbarsky [:bzbarsky]]

Gerv, the JS console already knows which errors it's getting are CSS errors and
which ones are JS errors.  It could easily filter them out as desired at display
time (the same way that it can already divide things into the
warnings/messages/errors categories and only display some subset).  Pretty much
anyone with a modicum of XUL programming knowledge should be able to implement that.

Wouldn't that be a more reasonable approach than complicating the back end with
extra prefs?  See also bug 228205 for more information on the way we would like
to move with the error console back end and ui....

Note that this is unlike dump() or strict warning support in that these are real
errors no different from JS errors, <map> parsing errors, security exceptions,
and the various other errors that are reported to the "JS Console" (which has
been itching to be renamed "Error Console" for years now).

Comment 2

[Gervase Markham [:gerv]]

bz: but CSS errors also appear in the "-console" console, don't they? And
there's no way to turn them off there. Which annoys me :-)

But fair enough. I just wanted to raise the idea.

Gerv

Comment 3

[Boris Zbarsky [:bzbarsky]]

> bz: but CSS errors also appear in the "-console" console, don't they?

Yes, in debug builds.  That could be fixed up so that the output to the text
console is done by a console service listener (instead of by raw printf).  Then
this listener could follow the JS console settings.

But that's debug builds.  "Real people" (other than the development team) don't
use debug builds.

Comment 4

[Jiri Znamenacek]

Attachment: modified JS-console related files

As a proof of concept I've hacked Error Console (err, JavaScript Console) in
Firefox (for-XForms build, Gecko/20050202) to provide menu for filtering
errors. Just replace appropriate files in toolkit.jar with ones from attached
zip-file and restart browser. (I don't use Firefox, I just this build to test
it ;)

I've left my debug code in, it's non-localizable and it looks ugly (I should
study XUL broadcaster first). But it seems to work. Currently it sorts "CSS
Parser" and "CSS Loader" error category under CSS filter and "XPConnect
JavaScript" and "content javascript" under JS filter. There are probably more
categories (XSLT?) but I don't know of any simple way of finding them out.

Thanks Boris for pointing me in the right direction. Comments welcome ^_-

Comment 5

[[e-mail]]

Is there any way of creating a dynamic CSS style to hide the rows that aren't
from a specific source? That would make it possible to create a menulist of all
discovered sources and quickly filter based on the selected item. Otherwise it
could still be done by manually looping through all of the rows and
showing/hiding them as appropriate, plus also checking when adding new rows.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> Is there any way of creating a dynamic CSS style to hide the rows that aren't
> from a specific source?

If the document has a stylesheet that can be edited via DOM, then yes.  See
http://w3.org/TR/2000/REC-DOM-Level-2-Style-20001113/css.html#CSS-CSSStyleSheet
the insertRule and deleteRule methods.

Comment 7

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

OK, due to the total lack of progress on the front end side, I'll make a pref.

Comment 8

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

Attachment: patch to add pref and change from error to warning

Comment 9

[Martijn Wargers (dead)]

Well, I made a front end patch (based on Jiri's code) in bug 275265. 

Comment 10

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 206269 [details] [diff] [review]
patch to add pref and change from error to warning

r=shaver!

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 206269 [details] [diff] [review]
patch to add pref and change from error to warning

>Index: layout/style/nsCSSScanner.cpp
>+#define CSS_ERRORS_PREF "layout.css.report_errors"
>+
>+PR_STATIC_CALLBACK(int) CSSErrorsPrefChanged(const char *aPref, void *aClosure)

Both the define and the function decl should be inside #ifdef CSS_REPORT_PARSE_ERRORS.

With that, sr=bzbarsky

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 206269 [details] [diff] [review]
patch to add pref and change from error to warning

sr=bzbarsky

Comment 13

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

Checked in to trunk.

Comment 14

[Sander]

Any chance of getting this on branch as well?

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 206269 [details] [diff] [review]
patch to add pref and change from error to warning

a- for 1.8.0.1, you don't (yet) need approvals for 1.8.1

Comment 16

[Nickolay_Ponomarev]

*** Bug 316200 has been marked as a duplicate of this bug. ***

Comment 17

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

Checked in to MOZILLA_1_8_BRANCH.

Comment 18

[Simon Bünzli]

*** Bug 350858 has been marked as a duplicate of this bug. ***