css error reporting should be pref-controlled

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
13 years ago
11 years ago

People

(Reporter: gerv, Assigned: dbaron)

Tracking

({fixed1.8.1})

Trunk
fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
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
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).
(Reporter)

Comment 2

13 years ago
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

> 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.
(Reporter)

Updated

13 years ago
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → WONTFIX

Comment 4

13 years ago
Created attachment 173965 [details]
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

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

Comment 7

12 years ago
OK, due to the total lack of progress on the front end side, I'll make a pref.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
(Assignee)

Comment 8

12 years ago
Created attachment 206269 [details] [diff] [review]
patch to add pref and change from error to warning
Attachment #206269 - Flags: superreview?(bzbarsky)
Attachment #206269 - Flags: review?
(Assignee)

Updated

12 years ago
Attachment #206269 - Flags: review? → review?(shaver)
Well, I made a front end patch (based on Jiri's code) in bug 275265. 
Comment on attachment 206269 [details] [diff] [review]
patch to add pref and change from error to warning

r=shaver!
Attachment #206269 - Flags: review?(shaver) → review+
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 on attachment 206269 [details] [diff] [review]
patch to add pref and change from error to warning

sr=bzbarsky
Attachment #206269 - Flags: superreview?(bzbarsky) → superreview+
(Assignee)

Comment 13

12 years ago
Checked in to trunk.
Status: REOPENED → RESOLVED
Last Resolved: 13 years ago12 years ago
Resolution: --- → FIXED

Comment 14

12 years ago
Any chance of getting this on branch as well?

Updated

12 years ago
Blocks: 275265

Updated

12 years ago
Attachment #206269 - Flags: approval1.8.1?
Attachment #206269 - Flags: approval1.8.0.1?
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
Attachment #206269 - Flags: approval1.8.0.1? → approval1.8.0.1-

Comment 16

12 years ago
*** Bug 316200 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

12 years ago
Whiteboard: [patch]

Updated

12 years ago
Attachment #206269 - Flags: approval1.8.1? → branch-1.8.1?(bzbarsky)
Attachment #206269 - Flags: branch-1.8.1?(bzbarsky) → branch-1.8.1+
(Assignee)

Comment 17

12 years ago
Checked in to MOZILLA_1_8_BRANCH.
Keywords: fixed1.8.1

Comment 18

11 years ago
*** Bug 350858 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.