Summary: | filterRootById accidentally clears inAdjacentChain flag | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||
Component: | CSS | Assignee: | 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
Yusuke Suzuki
2014-09-16 03:56:12 PDT
Created attachment 238172 [details]
Patch
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 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'); });` Committed r173688: <http://trac.webkit.org/changeset/173688> |