Bug 111957

Summary: Make sure that CSSSelector::setValue() is never called after parsing its pseudoType().
Product: WebKit Reporter: Hayato Ito <hayato>
Component: CSSAssignee: Hayato Ito <hayato>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, esprehn+autocc, macpherson, menard, ojan.autocc, webcomponents-bugzilla, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Add an assertion.
none
Add an assertion.
none
Patch for landing
none
Patch for landing none

Hayato Ito
Reported 2013-03-10 23:49:05 PDT
It'd be nice to have an assertion here since CSSParserSelector::pseudoType() has an side effect.
Attachments
Add an assertion. (1.44 KB, patch)
2013-03-10 23:52 PDT, Hayato Ito 		no flags
DetailsFormatted DiffDiff
Add an assertion. (1.43 KB, patch)
2013-03-10 23:56 PDT, Hayato Ito 		no flags
DetailsFormatted DiffDiff
Patch for landing (1.51 KB, patch)
2013-03-11 01:06 PDT, Hayato Ito 		no flags
DetailsFormatted DiffDiff
Patch for landing (1.51 KB, patch)
2013-03-11 01:15 PDT, Hayato Ito 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Hayato Ito
Comment 1 2013-03-10 23:52:56 PDT
Created attachment 192408 [details] Add an assertion.
Hayato Ito
Comment 2 2013-03-10 23:56:15 PDT
Created attachment 192409 [details] Add an assertion.
Hajime Morrita
Comment 3 2013-03-11 00:55:48 PDT
Comment on attachment 192409 [details] Add an assertion. View in context: https://bugs.webkit.org/attachment.cgi?id=192409&action=review > Source/WebCore/ChangeLog:9 > + CSSSelector::pseudoType() has an side effect. Could you elaborate? Tightening ASSERT()ion up is fine in general. But it's hard to see the connectio between pseudoType() and setValue(), and ASSERT() and the side effect.
Hayato Ito
Comment 4 2013-03-11 00:59:17 PDT
Thank you for the review. Let me land this after adding more description. (In reply to comment #3) > (From update of attachment 192409 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192409&action=review > > > Source/WebCore/ChangeLog:9 > > + CSSSelector::pseudoType() has an side effect. > > Could you elaborate? > Tightening ASSERT()ion up is fine in general. > But it's hard to see the connectio between pseudoType() and setValue(), and ASSERT() and the side effect.
Hayato Ito
Comment 5 2013-03-11 01:06:42 PDT
Created attachment 192418 [details] Patch for landing
Hayato Ito
Comment 6 2013-03-11 01:15:35 PDT
Created attachment 192420 [details] Patch for landing
WebKit Review Bot
Comment 7 2013-03-11 03:38:34 PDT
Comment on attachment 192420 [details] Patch for landing Clearing flags on attachment: 192420 Committed r145352: <http://trac.webkit.org/changeset/145352>
WebKit Review Bot
Comment 8 2013-03-11 03:38:38 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.