WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
141373
Add parsing support for CSS Selector L4's case-insensitive attribute
https://bugs.webkit.org/show_bug.cgi?id=141373
Summary
Add parsing support for CSS Selector L4's case-insensitive attribute
Benjamin Poulain
Reported
2015-02-08 13:08:23 PST
Add parsing support for CSS Selector L4's case-insensitive attribute
Attachments
Patch
(267.24 KB, patch)
2015-02-08 13:20 PST
,
Benjamin Poulain
darin
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Benjamin Poulain
Comment 1
2015-02-08 13:20:41 PST
Created
attachment 246247
[details]
Patch
Darin Adler
Comment 2
2015-02-08 16:21:43 PST
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.
Benjamin Poulain
Comment 3
2015-02-08 18:57:17 PST
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.
Benjamin Poulain
Comment 4
2015-02-08 19:45:00 PST
Committed
r179819
: <
http://trac.webkit.org/changeset/179819
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug