Closed
Bug 393757
Opened 17 years ago
Closed 16 years ago
Don't report CSS style errors for identifiers designated as vendor-specific
Categories
(Core :: CSS Parsing and Computation, enhancement)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha8
People
(Reporter: Waldo, Assigned: Waldo)
References
()
Details
Attachments
(2 files)
(deleted),
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Waldo
:
review+
Waldo
:
superreview+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
We could cut down on CSS error spam by not reporting errors for non-Gecko, vendor-specific identifiers which follow CSS2.1 rules. Consequently, I propose we not report errors for any identifier meeting the following criteria:
- begins with '-'/'_'
- but not with '-moz-', to enable typo-catching
The only down side I see is if someone mistypes the -moz- as -mzo- or similar (and won't get an error). I expect this occurs much less often than using other-vendor or IE-workaround identifiers, and I'd bet you're less likely to misread and mistype '-moz-' than most strings.
The following patch removes these errors for property names and at-rules. Note that nsICSSParser::ParseProperty doesn't need to be changed because it can only be called with an nsCSSProperty (and the only unhandled one is eCSSProperty_UNKNOWN). I've used the following URL as a testcase (should only get warnings for @-moz-e and -moz-e):
data:text/html,<style>@-moz-e{}@-khtml-f{}f{-moz-e:foo;-khtml-e:bar;_width:2px;-width:3px;}</style>
At a glance, doing this for other parts of CSS requires decentralized fixes or parser cleanup, so I'm leaving property values, pseudo-selectors, etc. for future patches.
Attachment #278288 -
Flags: superreview?(bzbarsky)
Attachment #278288 -
Flags: review?(bzbarsky)
Comment 1•17 years ago
|
||
This needs review from dbaron
Comment 2•17 years ago
|
||
If we do this then we should probably ignore mso- as well.
http://www.w3.org/TR/CSS21/syndata.html#vendor-keyword-history
Assignee | ||
Comment 3•17 years ago
|
||
Comment on attachment 278288 [details] [diff] [review]
Ignore other-vendor @at-rules and property names
Okay; I figured I'd try to avoid dbaron since he's out-of-townish for awhile. He was okay with the general idea when I mentioned it to him a few weeks ago, although I mentioned doing it only for identifiers matching /^-.+-/ but not /^-moz-/ because I hadn't read that section of CSS2.1 before.
As for mso-, I think we should only ignore what the spec says can be ignored. I doubt many not-saved-Word-docs use it, and this is mostly to help web developers.
Attachment #278288 -
Flags: superreview?(dbaron)
Attachment #278288 -
Flags: superreview?(bzbarsky)
Attachment #278288 -
Flags: review?(dbaron)
Attachment #278288 -
Flags: review?(bzbarsky)
Comment 4•17 years ago
|
||
Comment on attachment 278288 [details] [diff] [review]
Ignore other-vendor @at-rules and property names
>+ return (ident.First() == PRUnichar('-') || ident.First() == PRUnichar('_')) &&
>+ !StringBeginsWith(ident, NS_LITERAL_STRING("-moz-"));
You could put the "&& !StringBeginsWith()" inside the first half of the ||.
r+sr=dbaron
Sorry for the long delay.
Attachment #278288 -
Flags: superreview?(dbaron)
Attachment #278288 -
Flags: superreview+
Attachment #278288 -
Flags: review?(dbaron)
Attachment #278288 -
Flags: review+
Assignee | ||
Comment 5•17 years ago
|
||
Cuts down on error console spam for site authors that use IE hacks or include code tailored to the CSS support of other browsers...
Attachment #283496 -
Flags: superreview+
Attachment #283496 -
Flags: review+
Attachment #283496 -
Flags: approval1.9?
Updated•17 years ago
|
Attachment #283496 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 6•17 years ago
|
||
Comment on attachment 283496 [details] [diff] [review]
Address comment
Checked in; I'll get to the rest of the things eventually, although not likely soon given that the checks are currently spread throughout the parser and will either require a careful hand or (more likely) refactoring to make fewer checks necessary.
Comment 7•17 years ago
|
||
Comment on attachment 283496 [details] [diff] [review]
Address comment
Resetting all approval1.9+ flags on bugs that have not been checked in by Oct 22 11:59 PM PDT. Please re-request approval if needed.
Attachment #283496 -
Flags: approval1.9+
Comment 8•17 years ago
|
||
Comment on attachment 283496 [details] [diff] [review]
Address comment
Landed by Waldo on 2007-10-03 21:58.
Attachment #283496 -
Flags: approval1.9?
Comment 9•17 years ago
|
||
Comment on attachment 283496 [details] [diff] [review]
Address comment
Reset approval flag to + as it was already checked in.
Attachment #283496 -
Flags: approval1.9? → approval1.9+
Comment 10•16 years ago
|
||
Resolving per comment 9
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 11•16 years ago
|
||
I filed bug 433074 as followup to fix the remaining issues, although I'm not 100% sure what they all are.
You need to log in
before you can comment on or make changes to this bug.
Description
•