RESOLVED FIXED 136851
filterRootById accidentally clears inAdjacentChain flag
https://bugs.webkit.org/show_bug.cgi?id=136851
Summary filterRootById accidentally clears inAdjacentChain flag
Yusuke Suzuki
Reported 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.
Attachments
Patch (4.75 KB, patch)
2014-09-16 03:58 PDT, Yusuke Suzuki
benjamin: review+
Yusuke Suzuki
Comment 1 2014-09-16 03:58:25 PDT
Benjamin Poulain
Comment 2 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.
Yusuke Suzuki
Comment 3 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'); });`
Yusuke Suzuki
Comment 4 2014-09-17 01:55:07 PDT
Radar WebKit Bug Importer
Comment 5 2014-12-22 10:35:55 PST
Simon Fraser (smfr)
Comment 6 2014-12-22 15:10:56 PST
Note You need to log in before you can comment on or make changes to this bug.