Closed Bug 65469 Opened 24 years ago Closed 20 years ago

get CSS error reporting working

Categories

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

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Future

People

(Reporter: dbaron, Assigned: dbaron)

References

(Depends on 1 open bug)

Details

Attachments

(8 files)

get things working reasonably well...
19.86 KB, patch
Details | Diff | Splinter Review
current work
21.59 KB, patch
Details | Diff | Splinter Review
a few more improvements (better macro names and improvements in selector handling) - untested
24.33 KB, patch
Details | Diff | Splinter Review
the changes against a current tree
24.68 KB, patch
Details | Diff | Splinter Review
changes against a current tree (update due to jag's string changes)
24.67 KB, patch
Details | Diff | Splinter Review
changes against current tree (update due to glazman's changes)
24.70 KB, patch
Details | Diff | Splinter Review
Fix for not reporting "//" comments within declaration blocks.
910 bytes, patch
Details | Diff | Splinter Review
patch to enable
1.15 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review 
This is a bug to finish and perhaps enable (at least in DEBUG builds) CSS error
reporting.

I have a patch that makes it actually work pretty well, although there aren't
detailed errors for any of the value parsers - just "error parsing value for
property X".  I've gotten rid of the errors that weren't errors, and I've made
the scanner accumulate errors so we only print one report per error.  Patch
coming (although it's just a first installment).
And at some point I should make it learn the right numbers for CSS evaluated
from within HTML or JavaScript!
I need to figure out why it doesn't report "//" comments.
David: I assume you are going to move the strings into a properties file before 
checking in, right?
QA Contact: chrisd → ian
Certainly not until I get everything else working (since it's a lot easier to
understand the code if the strings are right there).  And even then this
might be hard.  And I might want to check in stages, since I may not get to
finishing this for a while.
BTW, I really don't like the design of this solution at all.  I'd greatly prefer
to pass errors as return values that indicated the error, except that would be
a bit more work, and also require modifing the CSS parser itself rather than
just adding code around the edges.  However, I may still try to do it...
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9
I think this current patch would be more useful for chrome developers than
what's currently in the tree.  However, pushing off to Mozilla 1.1 when I
may have time to rework the error handling in the CSS parser and do the whole
thing right.  If anyone thinks I should try to get what I have checked in,
give a yell.
Target Milestone: mozilla0.9 → mozilla1.1
OS: Linux → All
Hardware: PC → All
I'd either like to check in the improvements that I currently have to the CSS
error reporting or remove it from the tree.  I'd prefer the first because I
think this is a useful debugging tool for others, although others don't seem
to be using it.  At some point I'd hope to do this the right way, but that
would involve significant changes to the CSS parser.

Any thoughts?
David, I'd like to see the changes get in. I agree that it is a very nice
developer-feature. I'm wondering why you would *not* want to get it in?
I love it...   r=glazman

For the future, I suggest consistency in error messages with W3C CSS validator
Don't the error messages need to be localizable?
blake: As I've said above (2001-01-19 06:41), this is work in progress.  It's
not enabled yet, and it's a lot easier to work with when the strings are right
in front of me.  The reasons I want to check it in are:
 * I don't want to have tons of changes in my tree
 * It's a useful debugging feature that I want people who are writing chrome to
use

The whole thing is really a hack, and I'd prefer not to localize it until it's
written properly, which requires rewriting much of the CSS parser's error
handling.  And I'd rather not do that until CSS3's grammar is finished...
sr=attinasi. Thanks David.
Blocks: 6211
FWIW, the previous (large) patch was checked in 2001-03-05 17:57 PST.
glazou: Considering how useful the W3C CSS Validator error messages are, I would
rather rely on David's wording...
David, the change looks fine, except I don't know what this is:

+ * vim:ts=2:et:sw=2:

Do you want that there, and why have I not seen it in other files? I'm guessing
it does for VIm what the other line does for eMacs...

Regardless, [s]r=attinasi
Above patch was checked in with bug 83482.
Target Milestone: mozilla1.1 → Future
So... what's the status on this bug? It's been almost 3/4 of a year since the
patch was checked in. Has this bug been squashed for 8.5 months and not marked
as such?
More work needs to be done before it can be turned on.
Would CSS parser warnings hurt forward compatibility?
They would cause us to give errors on things in future versions of CSS -- but
they would be useful for web developers to see what it is that Mozilla doesn't
understand.  They wouldn't affect anything other than the actual warnings/errors
being displayed.
*** Bug 162425 has been marked as a duplicate of this bug. ***
Issues that should block this being turned on:
 * bug 187007 - localizability
 * getting the line numbers right for STYLE elements and STYLE attributes (via
HTML, XML, and XUL content sinks)
*** Bug 136158 has been marked as a duplicate of this bug. ***
Depends on: 147015
Depends on: 78179
Hmmm for line numbers, style elements are easy ("do what script does"), but
style attributes are somewhat harder.  Even if we fix them to work right during
parsing (which may need a change to the signature of SetAttr or at least some
sort of evil hackery), what line number should be reported when .style is set
via script?
bz: for dynamic style attribute setting, the setter might include "jsdbgapi.h"
and use JS_FrameIterator, JS_GetFramePC, and JS_PCToLineNumber; also
JS_GetFrameScript and JS_GetScriptFilename to good advantage.

/be
All that jsdbgapi.h cruft should of course be hidden under nsIScriptContext or
some such similar interface.

/be
Depends on: 240420
Depends on: 240421
Attached patch patch to enable — — Splinter Review 
The minimo XXX comment is for whenever somebody goes through to find all the
little things in layout that could be turned off for minimo -- there's no
particular reason to do this one now and not others...
Attachment #159509 - Flags: superreview?(bzbarsky)
Attachment #159509 - Flags: review?(bzbarsky)
Comment on attachment 159509 [details] [diff] [review]
patch to enable

r+sr=bzbarsky
Attachment #159509 - Flags: superreview?(bzbarsky)
Attachment #159509 - Flags: superreview+
Attachment #159509 - Flags: review?(bzbarsky)
Attachment #159509 - Flags: review+
Fix checked in to trunk, 2004-09-20 21:41 -0700.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Depends on: 261337
Depends on: 261339
It would be nice to be able to switch CSS errors off, or filter them. Important
javascript errors are overseen within hundrets of them...
(In reply to comment #38)
> It would be nice to be able to switch CSS errors off, or filter them. Important
> javascript errors are overseen within hundrets of them...

See bug 264162 - css error reporting should be pref-controlled.
Bug 264162 is about the client (=browser) side.  But what if the server side
gets a chance to control that?

I can imagine webmasters are including proprietary properties in their CSS to
get around the trouble of maintaining multiple user-agent specific CSSs. 
Wouldn't be a Mozilla specific CSS instruction along the line...
"If user-agent == Mozilla then skip next property"
...helpful, without conflicting with W3C philosophy or evangelism?

I'm aware that this may sound a bit pointless since unknown properties are
skipped anyway.
Just thinking about a way not to let CSS writers look like **** coders while
they in reality just want to maintain cross-browser compatibility.
No longer depends on: 261337
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: