CSSLint / parser-lib

Collection of parsers written in JavaScript
http://www.nczonline.net/
Other
287 stars 82 forks source link

error when "css \9 hack " #21

Open dongyuwei opened 12 years ago

dongyuwei commented 12 years ago
body {  
 color: red; /* all browsers, of course */  
 color : green\9; /* IE8 and below */  
}  
nzakas commented 12 years ago

What is the error? And what do you think it should do?

dongyuwei commented 12 years ago
var parserlib = require("parserlib");//npm install -g parserlib

var parser = new parserlib.css.Parser({
    starHack : true,
    underscoreHack : true,
    ieFilters: true,
    strict : false
});

parser.addListener("error", function(event){
    console.log("Parse error: " + event.message + " (" + event.line + "," + event.col + ")", "error");
    console.error('\033[91m' + event.message  + event.line + "," + event.col+ '\033[0m');
    console.log(event);
});

var css = '.W_input{padding:0 0 0 2px; padding:4px 0 0 2px\9;vertical-align:top; color:#666;}';
//css = css2.replace(/\\9/g,'');
var ast = parser.parse(css);
//-------------------------------------------------------------------------------//
//node test-css-parserlib.js

Parse error: Expected RBRACE at line 1, col 45. (1,45) error Expected RBRACE at line 1, col 45.1,45 { type: 'error', error: { col: 45, line: 1, message: 'Expected RBRACE at line 1, col 45.' }, message: 'Expected RBRACE at line 1, col 45.', line: 1, col: 45 }

dongyuwei commented 12 years ago

I think you can support another hack option paramter for the Parser constructor.

eitanmk commented 12 years ago

@nzakas I'm curious what you think with regard to support for tolerating all the browser hacks. I'm torn between asking for support versus keeping the parser true to the CSS spec.

Here's a list from Paul Irish: http://paulirish.com/2009/browser-specific-css-hacks/

I was thinking of suggesting to consolide tolerance for all hacks into one option (either consider browser hacks a syntax error, or tolerate them) and make the presence of a hack fire a new event for handling downstream. Something like that would be useful for CSSLint. However, I can also see how it complicates the parser. Though these hacks seem to have longevity once introduced, it seems like a bit of a chore to have to update the parser logic based on a browser release instead of a CSS spec update.

nzakas commented 12 years ago

Generally I think it's good to be tolerant of hacks that browsers are tolerant of. I've gone with the convention of one option per hack, but right now just have star and underscore hacks. \9 seems like a good one to add as well, but I'm unsure of some of the others.

PlNG commented 11 years ago

If they're using inline CSS hacks for IE, they're doing it wrong. They should be using conditional comments to serve a separate stylesheet that gets ignored by other browsers but gets parsed by IE all the way back to IE5.

nschonni commented 11 years ago

The \9 trick isn't just for old IE, it is useful for IE9 (not sure about 10) after you have already done you separate sheets for IE9<

suihan commented 10 years ago

is this issue be fixed?

nzakas commented 10 years ago

No

stubbornella commented 10 years ago

I think we should add /9 to the list of allowed hacks. That said, it is a bit complicated because I believe you can put anything after the /, not just a 9. However, maybe we should consider only allowing /9 rather than trying to accommodate the entire spectrum.

cscott commented 8 years ago

I suspect the latest round of escaping hacks actually makes this "work" -- that is, the green\9 is parsed as a proper identifier (with unicode character 0009 at the end), and then you'd probably get a complaint that green\9 isn't a valid color name... which is correct... for everyone except IE9 I guess? But you shouldn't get a syntax error here any more, I don't think.

I think something like /* csslint ignore:line */ is probably the most reasonable solution to accomodate the long tail of browser-specific CSS hacks.

ideadapt commented 8 years ago

Just for completion, these error seems still to be around, at least on current beta branch:

3681: error at line 8488, col 18
Unexpected token '1.125rem  ' at line 8488, col 18.
  margin-bottom: 1.125rem\9;

I will remove the \9 hack soon anyway ..., but its still reported though.