Add parsing support for CSS Selector L4's case-insensitive attribute
Created attachment 246247 [details] Patch
Comment on attachment 246247 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246247&action=review > Source/WebCore/css/CSSGrammar.y.in:1297 > + if (UNLIKELY($1.length() != 1)) > + YYERROR; > + > + UChar character = $1[0]; > + if (LIKELY(character == 'i' || character == 'I')) > + $$ = true; > + else > + YYERROR; We have a more efficient function for this in ASCIICType.h, named isASCIIAlphaCaselessEqual. I would write: if (UNLIKELY($1.length() != 1 || !isASCIIAlphaCaselessEqual($1[0], 'i'))) YYERROR; $$ = true; Is YYERROR exactly what we want here? Do we have enough test coverage to see if our error recovery prevents us from discarding rules other than the one with the badly formed selector? > Source/WebCore/css/CSSSelector.cpp:-664 > - str.append(']'); Was this a bug? Do we have test coverage? > Source/WebCore/css/CSSSelector.cpp:687 > + str.append(" i]"); Should be appendLiteral.
Thanks for the review! (In reply to comment #2) > We have a more efficient function for this in ASCIICType.h, named > isASCIIAlphaCaselessEqual. > > I would write: > > if (UNLIKELY($1.length() != 1 || !isASCIIAlphaCaselessEqual($1[0], 'i'))) > YYERROR; > $$ = true; Yep, that's a good idea. > Is YYERROR exactly what we want here? Do we have enough test coverage to see > if our error recovery prevents us from discarding rules other than the one > with the badly formed selector? Invalid selector is quite destructive, the entire rule is discarded. There are two levels of recovery that ensure we nuke everything. I can add a little test for that. > > Source/WebCore/css/CSSSelector.cpp:-664 > > - str.append(']'); > > Was this a bug? Do we have test coverage? This did not change the behavior, ']' is added below in the general path. I'll put this back, I did not intend to be confusing.
Committed r179819: <http://trac.webkit.org/changeset/179819>