Bug 136851

Summary: filterRootById accidentally clears inAdjacentChain flag
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: CSSAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch benjamin: review+

Description Yusuke Suzuki 2014-09-16 03:56:12 PDT
When SubSelector comes, inAdjacentChain flag is cleared to false.
So for example, `document.querySelector("span#id + ok")` doesn't work correctly.
Comment 1 Yusuke Suzuki 2014-09-16 03:58:25 PDT
Created attachment 238172 [details]
Patch
Comment 2 Benjamin Poulain 2014-09-16 12:43:26 PDT
Comment on attachment 238172 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238172&action=review

Good catch and good fix.

> Source/WebCore/ChangeLog:11
> +        * dom/SelectorQuery.cpp:
> +        (WebCore::filterRootById):

You could add your copyright to this file, you already fixed a bunch of stuff here.

> LayoutTests/fast/selectors/filter-root-node-with-selector-contains-adjacents.html:35
> +shouldBe('document.querySelectorAll("span#first + t1").length', '1');
> +shouldBe('document.querySelectorAll("span#second ~ t2").length', '1');
> +shouldBe('document.querySelectorAll("span#third + t3 > t4").length', '1');
> +shouldBe('document.querySelectorAll("span#third + t3 > t5").length', '0');
> +shouldBe('document.querySelectorAll("span#fourth t6 ~ t7").length', '3');

Can you please extend those test cases to sub-selectors on the left and on the right to cover all possible cases?

Something like "span#first + t1", ".class#first + t1", ""#first.class + t1",  "span#first.class + t1", etc.
Comment 3 Yusuke Suzuki 2014-09-17 01:16:27 PDT
Comment on attachment 238172 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238172&action=review

Thank you for your review, Benjamin!

>> Source/WebCore/ChangeLog:11
>> +        (WebCore::filterRootById):
> 
> You could add your copyright to this file, you already fixed a bunch of stuff here.

Thanks! I've added my copyright.

>> LayoutTests/fast/selectors/filter-root-node-with-selector-contains-adjacents.html:35
>> +shouldBe('document.querySelectorAll("span#fourth t6 ~ t7").length', '3');
> 
> Can you please extend those test cases to sub-selectors on the left and on the right to cover all possible cases?
> 
> Something like "span#first + t1", ".class#first + t1", ""#first.class + t1",  "span#first.class + t1", etc.

Yes. I'll expand it by using the code such as
`[ 'span$', '.class$', '$.class', 'span$.class' ].map(function (p) { p.replace('$', '#id'); });`
Comment 4 Yusuke Suzuki 2014-09-17 01:55:07 PDT
Committed r173688: <http://trac.webkit.org/changeset/173688>
Comment 5 Radar WebKit Bug Importer 2014-12-22 10:35:55 PST
<rdar://problem/19326698>
Comment 6 Simon Fraser (smfr) 2014-12-22 15:10:56 PST
<rdar://problem/18824663>