136851 2014-09-16 03:56:12 -0700 filterRootById accidentally clears inAdjacentChain flag 2014-12-22 15:10:56 -0800 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 ysuzuki ysuzuki benjamin simon.fraser webkit-bug-importer oldest_to_newest 1035439 0 ysuzuki 2014-09-16 03:56:12 -0700 When SubSelector comes, inAdjacentChain flag is cleared to false. So for example, `document.querySelector("span#id + ok")` doesn't work correctly. 1035440 1 238172 ysuzuki 2014-09-16 03:58:25 -0700 Created attachment 238172 Patch 1035543 2 238172 benjamin 2014-09-16 12:43:26 -0700 Comment on attachment 238172 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. 1035683 3 238172 ysuzuki 2014-09-17 01:16:27 -0700 Comment on attachment 238172 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'); });` 1035687 4 ysuzuki 2014-09-17 01:55:07 -0700 Committed r173688: <http://trac.webkit.org/changeset/173688> 1057397 5 webkit-bug-importer 2014-12-22 10:35:55 -0800 <rdar://problem/19326698> 1057461 6 simon.fraser 2014-12-22 15:10:56 -0800 <rdar://problem/18824663> 238172 2014-09-16 03:58:25 -0700 2014-09-17 01:16:27 -0700 Patch bug-136851-20140916195848.patch text/plain 4868 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMTczNjM4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNGQ4YjQ5NDExNzY5MjMw MDA1NTc3ZjMyYzQ0YTljYzNmNWM0MGZiZi4uYzNjZjA5ZmZiYmU3NzUzMDhkNjc3OWRiY2VmNWVl YzBmNzA0N2NlMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE0LTA5LTE2ICBZdXN1 a2UgU3V6dWtpICA8dXRhdGFuZS50ZWFAZ21haWwuY29tPgorCisgICAgICAgIGZpbHRlclJvb3RC eUlkIGFjY2lkZW50YWxseSBjbGVhcnMgaW5BZGphY2VudENoYWluIGZsYWcKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzNjg1MQorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRlc3Q6IGZhc3Qvc2VsZWN0b3Jz L2ZpbHRlci1yb290LW5vZGUtd2l0aC1zZWxlY3Rvci1jb250YWlucy1hZGphY2VudHMuaHRtbAor CisgICAgICAgICogZG9tL1NlbGVjdG9yUXVlcnkuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Zmls dGVyUm9vdEJ5SWQpOgorCiAyMDE0LTA5LTE1ICBKb24gSG9uZXljdXR0ICA8amhvbmV5Y3V0dEBh cHBsZS5jb20+CiAKICAgICAgICAgTmFtaW5nIGNsZWFuLXVwLgpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkNvcmUvZG9tL1NlbGVjdG9yUXVlcnkuY3BwIGIvU291cmNlL1dlYkNvcmUvZG9tL1NlbGVj dG9yUXVlcnkuY3BwCmluZGV4IDIzOWIxYTUwMGZlYzE0Nzg0ZjQ1MzZhY2I3ZmIwM2E1ZGNlOGI4 MTUuLjU1OWRjODg1NDc0MzNlM2M2ZjUxZDZlYmQzNTZkMmNkNTYzN2FjZGEgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJDb3JlL2RvbS9TZWxlY3RvclF1ZXJ5LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29y ZS9kb20vU2VsZWN0b3JRdWVyeS5jcHAKQEAgLTI0Myw2ICsyNDMsOCBAQCBBTFdBWVNfSU5MSU5F IHZvaWQgU2VsZWN0b3JEYXRhTGlzdDo6ZXhlY3V0ZUZhc3RQYXRoRm9ySWRTZWxlY3Rvcihjb25z dCBDb250YWluZQogICAgICAgICAgICAgICAgIH0KICAgICAgICAgICAgIH0KICAgICAgICAgfQor ICAgICAgICBpZiAoc2VsZWN0b3ItPnJlbGF0aW9uKCkgPT0gQ1NTU2VsZWN0b3I6OlN1YlNlbGVj dG9yKQorICAgICAgICAgICAgY29udGludWU7CiAgICAgICAgIGlmIChzZWxlY3Rvci0+cmVsYXRp b24oKSA9PSBDU1NTZWxlY3Rvcjo6RGlyZWN0QWRqYWNlbnQgfHwgc2VsZWN0b3ItPnJlbGF0aW9u KCkgPT0gQ1NTU2VsZWN0b3I6OkluZGlyZWN0QWRqYWNlbnQpCiAgICAgICAgICAgICBpbkFkamFj ZW50Q2hhaW4gPSB0cnVlOwogICAgICAgICBlbHNlCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9D aGFuZ2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggNzI2ZjU1OTYxMjgxNzQ2ODY5 NzkwOGYzMzdjYjkzYjRmYTc2OWZjNy4uNzVlMTAxZThkM2Q0YTQ4YjU3NTliYjFlYWFmYjk4N2Nh NWMyOWM4OCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRl c3RzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE0LTA5LTE2ICBZdXN1a2UgU3V6dWtp ICA8dXRhdGFuZS50ZWFAZ21haWwuY29tPgorCisgICAgICAgIGZpbHRlclJvb3RCeUlkIGFjY2lk ZW50YWxseSBjbGVhcnMgaW5BZGphY2VudENoYWluIGZsYWcKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzNjg1MQorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFdoZW4gU3ViU2VsZWN0b3IgY29tZXMsIGBpbkFk amFjZW50Q2hhaW5gIGZsYWcgaXMgY2xlYXJlZCB0byBmYWxzZS4KKyAgICAgICAgU28gZm9yIGV4 YW1wbGUsIGBkb2N1bWVudC5xdWVyeVNlbGVjdG9yKCJzcGFuI2lkICsgb2siKWAgZG9lc24ndCB3 b3JrIGNvcnJlY3RseS4KKyAgICAgICAgSW4gdGhpcyBwYXRjaCwgd2UgaGFuZGxlcyBTdWJTZWxl Y3RvciByZWxhdGlvbiBjb3JyZWN0bHk7IGRvbid0IGNoYW5nZSBgaW5BZGphY2VudENoYWluYCBm bGFnCisgICAgICAgIHRvIGZpbHRlciB0aGUgcmlnaHQgc2VhcmNoIHJvb3Qgbm9kZS4KKworICAg ICAgICAqIGZhc3Qvc2VsZWN0b3JzL2ZpbHRlci1yb290LW5vZGUtd2l0aC1zZWxlY3Rvci1jb250 YWlucy1hZGphY2VudHMtZXhwZWN0ZWQudHh0OiBBZGRlZC4KKyAgICAgICAgKiBmYXN0L3NlbGVj dG9ycy9maWx0ZXItcm9vdC1ub2RlLXdpdGgtc2VsZWN0b3ItY29udGFpbnMtYWRqYWNlbnRzLmh0 bWw6IEFkZGVkLgorCiAyMDE0LTA5LTE1ICBSb2dlciBGb25nICA8cm9nZXJfZm9uZ0BhcHBsZS5j b20+CiAKICAgICAgICAgVW5yZXZpZXdlZC4gU2tpcCBpbml0aWFsLWxldHRlciB0ZXN0cyBvbiBX aW5kb3dzLgpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZmFzdC9zZWxlY3RvcnMvZmlsdGVyLXJv b3Qtbm9kZS13aXRoLXNlbGVjdG9yLWNvbnRhaW5zLWFkamFjZW50cy1leHBlY3RlZC50eHQgYi9M YXlvdXRUZXN0cy9mYXN0L3NlbGVjdG9ycy9maWx0ZXItcm9vdC1ub2RlLXdpdGgtc2VsZWN0b3It Y29udGFpbnMtYWRqYWNlbnRzLWV4cGVjdGVkLnR4dApuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRl eCAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwLi4yZDc4YTY1NGVjY2I5 ZTFkNzhjOGYzZTJlNDRhMGJjNjEzNjZhODE0Ci0tLSAvZGV2L251bGwKKysrIGIvTGF5b3V0VGVz dHMvZmFzdC9zZWxlY3RvcnMvZmlsdGVyLXJvb3Qtbm9kZS13aXRoLXNlbGVjdG9yLWNvbnRhaW5z LWFkamFjZW50cy1leHBlY3RlZC50eHQKQEAgLTAsMCArMSwxNCBAQAorVGVzdCBmaWx0ZXJpbmcg dGhlIHJvb3Qgbm9kZXMgYnkgbGV2ZXJhZ2luZyAjaWQgaW5zaWRlIHNlbGVjdG9ycyB3aXRoIGFk amFjZW50cy4KKworT24gc3VjY2VzcywgeW91IHdpbGwgc2VlIGEgc2VyaWVzIG9mICJQQVNTIiBt ZXNzYWdlcywgZm9sbG93ZWQgYnkgIlRFU1QgQ09NUExFVEUiLgorCisKK1BBU1MgZG9jdW1lbnQu cXVlcnlTZWxlY3RvckFsbCgic3BhbiNmaXJzdCArIHQxIikubGVuZ3RoIGlzIDEKK1BBU1MgZG9j dW1lbnQucXVlcnlTZWxlY3RvckFsbCgic3BhbiNzZWNvbmQgfiB0MiIpLmxlbmd0aCBpcyAxCitQ QVNTIGRvY3VtZW50LnF1ZXJ5U2VsZWN0b3JBbGwoInNwYW4jdGhpcmQgKyB0MyA+IHQ0IikubGVu Z3RoIGlzIDEKK1BBU1MgZG9jdW1lbnQucXVlcnlTZWxlY3RvckFsbCgic3BhbiN0aGlyZCArIHQz ID4gdDUiKS5sZW5ndGggaXMgMAorUEFTUyBkb2N1bWVudC5xdWVyeVNlbGVjdG9yQWxsKCJzcGFu I2ZvdXJ0aCB0NiB+IHQ3IikubGVuZ3RoIGlzIDMKK1BBU1Mgc3VjY2Vzc2Z1bGx5UGFyc2VkIGlz IHRydWUKKworVEVTVCBDT01QTEVURQorCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9mYXN0L3Nl bGVjdG9ycy9maWx0ZXItcm9vdC1ub2RlLXdpdGgtc2VsZWN0b3ItY29udGFpbnMtYWRqYWNlbnRz Lmh0bWwgYi9MYXlvdXRUZXN0cy9mYXN0L3NlbGVjdG9ycy9maWx0ZXItcm9vdC1ub2RlLXdpdGgt c2VsZWN0b3ItY29udGFpbnMtYWRqYWNlbnRzLmh0bWwKbmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5k ZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMC4uODcwN2JlNGZkNWJh ODM4MTQ2ZWYyZTMwMjUxNjA0ZDlmM2E3ZWJhZAotLS0gL2Rldi9udWxsCisrKyBiL0xheW91dFRl c3RzL2Zhc3Qvc2VsZWN0b3JzL2ZpbHRlci1yb290LW5vZGUtd2l0aC1zZWxlY3Rvci1jb250YWlu cy1hZGphY2VudHMuaHRtbApAQCAtMCwwICsxLDM4IEBACis8IWRvY3R5cGUgaHRtbD4KKzxodG1s PgorPGhlYWQ+Cis8c2NyaXB0IHNyYz0iLi4vLi4vcmVzb3VyY2VzL2pzLXRlc3QtcHJlLmpzIj48 L3NjcmlwdD4KKzwvaGVhZD4KKzxib2R5PgorPGRpdiBzdHlsZT0iZGlzcGxheTpub25lIj4KKyAg ICA8c3BhbiBpZD0iZmlyc3QiPjwvc3Bhbj4KKyAgICA8dDE+PC90MT4KKworICAgIDxzcGFuIGlk PSJzZWNvbmQiPjwvc3Bhbj4KKyAgICA8dDI+PC90Mj4KKworICAgIDxzcGFuIGlkPSJ0aGlyZCI+ PC9zcGFuPgorICAgIDx0Mz48dDQ+PC90ND48L3QzPgorCisgICAgPHNwYW4gaWQ9ImZvdXJ0aCI+ CisgICAgICAgIDx0Nj4KKyAgICAgICAgPC90Nj4KKyAgICAgICAgPHQ3PjwvdDc+CisgICAgICAg IDx0Nz48L3Q3PgorICAgICAgICA8dDc+CisgICAgICAgICAgICA8dDc+PC90Nz4KKyAgICAgICAg PC90Nz4KKyAgICA8L3NwYW4+Cis8L2Rpdj4KKzwvYm9keT4KKzxzY3JpcHQ+CitkZXNjcmlwdGlv bignVGVzdCBmaWx0ZXJpbmcgdGhlIHJvb3Qgbm9kZXMgYnkgbGV2ZXJhZ2luZyAjaWQgaW5zaWRl IHNlbGVjdG9ycyB3aXRoIGFkamFjZW50cy4nKTsKKworc2hvdWxkQmUoJ2RvY3VtZW50LnF1ZXJ5 U2VsZWN0b3JBbGwoInNwYW4jZmlyc3QgKyB0MSIpLmxlbmd0aCcsICcxJyk7CitzaG91bGRCZSgn ZG9jdW1lbnQucXVlcnlTZWxlY3RvckFsbCgic3BhbiNzZWNvbmQgfiB0MiIpLmxlbmd0aCcsICcx Jyk7CitzaG91bGRCZSgnZG9jdW1lbnQucXVlcnlTZWxlY3RvckFsbCgic3BhbiN0aGlyZCArIHQz ID4gdDQiKS5sZW5ndGgnLCAnMScpOworc2hvdWxkQmUoJ2RvY3VtZW50LnF1ZXJ5U2VsZWN0b3JB bGwoInNwYW4jdGhpcmQgKyB0MyA+IHQ1IikubGVuZ3RoJywgJzAnKTsKK3Nob3VsZEJlKCdkb2N1 bWVudC5xdWVyeVNlbGVjdG9yQWxsKCJzcGFuI2ZvdXJ0aCB0NiB+IHQ3IikubGVuZ3RoJywgJzMn KTsKKzwvc2NyaXB0PgorPHNjcmlwdCBzcmM9Ii4uLy4uL3Jlc291cmNlcy9qcy10ZXN0LXBvc3Qu anMiPjwvc2NyaXB0PgorPC9odG1sPgo=