137378 2014-10-02 20:04:06 -0700 Web Inspector: Highlighted selectors in Rules sidebar break with selectors that contain nested selector lists 2014-10-06 18:55:25 -0700 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck benjamin benjamin graouts joepeck kling timothy webkit-bug-importer oldest_to_newest 1039103 0 joepeck 2014-10-02 20:04:06 -0700 * TEST <style> .testClass, a,span , :nth-child(n of *, p), bumble, p { color: blue; } </style> <p class="testClass">Test</p> * STEPS TO REPRODUCE 1. Inspect the <p> on the test page 2. Open Styles Sidebar to the Rules section => in the Rules section ".testClass", ":nth-child(n of *", and "bumble" are highlighted as matching this particular element => expected ".testClass", ":nth-child(n of *, p)" and "p" 1039104 1 webkit-bug-importer 2014-10-02 20:04:29 -0700 <rdar://problem/18535308> 1039107 2 joepeck 2014-10-02 20:09:39 -0700 Nested selector lists are causing the CSSParser SourceData to be out of whack. On the protocol, we see the list of selectors is broken at the comma of the nested list: "selectorList": { "selectors": [".testClass", "a", "span", ":nth-child(n of *", "p)", "bumble", "p"], "text": ".testclass, a, span, :nth-child(n of *, p), bumble, p", ... } This is because CSSParser is marking the start/ends as it sees commas: (CSSGrammar.y.in) at_rule_header_end: { parser->markRuleHeaderEnd(); } ; at_selector_end: { parser->markSelectorEnd(); } ; selector_list: ... | selector_list at_selector_end ',' maybe_space before_selector_group_item complex_selector %prec UNIMPORTANT_TOK { ... So if we are doing a nested selector list, we get an errant early markSelectorEnd(). The markStart/End is only used by the Inspector here, so that we can send the exact text from the stylesheet to the frontend. We want this to show the case of the selector. ".testClass" instead of ".testclass". (InspectorStyleSheet.cpp) static PassRefPtr<Inspector::Protocol::Array<String>> selectorsFromSource(const CSSRuleSourceData* sourceData, const String& sheetText) { DEPRECATED_DEFINE_STATIC_LOCAL(JSC::Yarr::RegularExpression, comment, ("/\\*[^]*?\\*/", TextCaseSensitive, JSC::Yarr::MultilineEnabled)); RefPtr<Inspector::Protocol::Array<String>> result = Inspector::Protocol::Array<String>::create(); > const SelectorRangeList& ranges = sourceData->selectorRanges; for (size_t i = 0, size = ranges.size(); i < size; ++i) { const SourceRange& range = ranges.at(i); String selector = sheetText.substring(range.start, range.length()); // We don't want to see any comments in the selector components, only the meaningful parts. replace(selector, comment, ""); result->addItem(selector.stripWhiteSpace()); } return result.release(); } PassRefPtr<Inspector::Protocol::CSS::SelectorList> InspectorStyleSheet::buildObjectForSelectorList(CSSStyleRule* rule) { ... > if (sourceData) > selectors = selectorsFromSource(sourceData.get(), m_parsedStyleSheet->text()); > else { selectors = Inspector::Protocol::Array<String>::create(); const CSSSelectorList& selectorList = rule->styleRule()->selectorList(); for (const CSSSelector* selector = selectorList.first(); selector; selector = CSSSelectorList::next(selector)) selectors->addItem(selector->selectorText()); } ... } 1039441 3 239260 benjamin 2014-10-03 20:16:45 -0700 Created attachment 239260 Patch 1039773 4 239260 joepeck 2014-10-06 11:38:15 -0700 Comment on attachment 239260 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239260&action=review Looks good to me! May want another reviewer though. > Source/WebCore/ChangeLog:10 > + To make implement this with low risk of unbalanced start+end and good error recovery, Grammaro: "To make implement" => "To implement" 1039858 5 239260 joepeck 2014-10-06 15:19:13 -0700 Comment on attachment 239260 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239260&action=review > Source/WebCore/css/CSSParser.h:378 > + unsigned m_nestedSelectorLevel; Nit: You might be able to move this member up next to m_numParsedPropertiesBeforeMarginBox and it might save a few bytes in the size of CSSParser. Not important at all though. 1039915 6 benjamin 2014-10-06 18:55:25 -0700 Committed r174379: <http://trac.webkit.org/changeset/174379> 239260 2014-10-03 20:16:45 -0700 2014-10-06 15:19:13 -0700 Patch bug-137378-20141003201642.patch text/plain 5719 benjamin U3VidmVyc2lvbiBSZXZpc2lvbjogMTc0MzA0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZmU0YzVlY2Y4NTZmMzU5 ZWI3YzE4Y2ZmOGVlMDliYzMxMmRlZjNlZS4uOTRkM2E4NzJiOGFmNWYzMTBjYTE0N2Q4NjVkZTVi MzU3NzEzZDUyZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI3IEBACisyMDE0LTEwLTAzICBCZW5q YW1pbiBQb3VsYWluICA8YnBvdWxhaW5AYXBwbGUuY29tPgorCisgICAgICAgIFdlYiBJbnNwZWN0 b3I6IEhpZ2hsaWdodGVkIHNlbGVjdG9ycyBpbiBSdWxlcyBzaWRlYmFyIGJyZWFrIHdpdGggc2Vs ZWN0b3JzIHRoYXQgY29udGFpbiBuZXN0ZWQgc2VsZWN0b3IgbGlzdHMKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzNzM3OAorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFdlIHNob3VsZCBub3QgdXBkYXRlIHRo ZSBwYXJzZXIncyBzZWxlY3RvclJhbmdlIHdoZW4gbWF0Y2hpbmcgYSBuZXN0ZWQgc2VsZWN0b3Ig bGlzdC4KKworICAgICAgICBUbyBtYWtlIGltcGxlbWVudCB0aGlzIHdpdGggbG93IHJpc2sgb2Yg dW5iYWxhbmNlZCBzdGFydCtlbmQgYW5kIGdvb2QgZXJyb3IgcmVjb3ZlcnksCisgICAgICAgIEkg aGF2ZSB3cmFwcGVkIHRoZSBwYXJzaW5nIG9mIHNlbGVjdG9yX2xpc3QgYmV0d2VlbiBhIHN0YXJ0 L2VuZCBmdW5jdGlvbiBhdCB0aGUgZ3JhbW1hcgorICAgICAgICBsZXZlbC4gV2hlbmV2ZXIgdGhl IHBhcnNlciBlbnRlciBhIG5lc3RlZCBzZWxlY3RvciBsaXN0LCBtX25lc3RlZFNlbGVjdG9yTGV2 ZWwgZ29lcyB1cAorICAgICAgICBieSBvbmUuIEFueSB0aW1lIHRoZSBwYXJzaW5nIGlzIGN1cnJl bnRseSBpbiBhIG5lc3RlZCBzZWxlY3RvciBsaXN0LCB3ZSBuZXZlciB1cGRhdGUKKyAgICAgICAg dGhlIHNlbGVjdG9yIHJhbmdlcy4KKworICAgICAgICAqIGNzcy9DU1NHcmFtbWFyLnkuaW46Cisg ICAgICAgICogY3NzL0NTU1BhcnNlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpDU1NQYXJzZXI6 OkNTU1BhcnNlcik6CisgICAgICAgIChXZWJDb3JlOjpDU1NQYXJzZXI6Om1hcmtTZWxlY3RvclN0 YXJ0KToKKyAgICAgICAgKFdlYkNvcmU6OkNTU1BhcnNlcjo6bWFya1NlbGVjdG9yRW5kKToKKyAg ICAgICAgKiBjc3MvQ1NTUGFyc2VyLmg6CisgICAgICAgIChXZWJDb3JlOjpDU1NQYXJzZXI6OnN0 YXJ0TmVzdGVkU2VsZWN0b3JMaXN0KToKKyAgICAgICAgKFdlYkNvcmU6OkNTU1BhcnNlcjo6ZW5k TmVzdGVkU2VsZWN0b3JMaXN0KToKKwogMjAxNC0xMC0wMyAgQ2hyaXN0b3BoZSBEdW1leiAgPGNk dW1lekBhcHBsZS5jb20+CiAKICAgICAgICAgVXNlIGlzPD4oKSAvIGRvd25jYXN0PD4oKSBmb3Ig Q1NTVmFsdWUgc3ViY2xhc3NlcwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvY3NzL0NTU0dy YW1tYXIueS5pbiBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NHcmFtbWFyLnkuaW4KaW5kZXggZTgy NTZlMDhkNTZjOTJiZDg5Yjg5OTJhNTQyMTFlNGU5OGJlMzFmMy4uMjliYjQ4NmY2YTE3YWU1MDQy NWE2NjkzMTM2OGUwYjZiNzZiNmNhOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvY3NzL0NT U0dyYW1tYXIueS5pbgorKysgYi9Tb3VyY2UvV2ViQ29yZS9jc3MvQ1NTR3JhbW1hci55LmluCkBA IC0zMDksOCArMzA5LDggQEAgc3RhdGljIGJvb2wgaXNWYWxpZE50aFNlbGVjdG9yTGlzdChjb25z dCBWZWN0b3I8c3RkOjp1bmlxdWVfcHRyPENTU1BhcnNlclNlbGVjdG8KICVkZXN0cnVjdG9yIHsg ZGVsZXRlICQkOyB9IGF0dHJpYiBjbGFzcyBwYWdlX3NlbGVjdG9yIHBzZXVkbyBwc2V1ZG9fcGFn ZSBjb21wbGV4X3NlbGVjdG9yIGNvbXBsZXhfc2VsZWN0b3Jfd2l0aF90cmFpbGluZ193aGl0ZXNw YWNlIGNvbXBvdW5kX3NlbGVjdG9yIHNwZWNpZmllciBzcGVjaWZpZXJfbGlzdAogCiAldW5pb24g eyBWZWN0b3I8c3RkOjp1bmlxdWVfcHRyPENTU1BhcnNlclNlbGVjdG9yPj4qIHNlbGVjdG9yTGlz dDsgfQotJXR5cGUgPHNlbGVjdG9yTGlzdD4gc2VsZWN0b3JfbGlzdCBzaW1wbGVfc2VsZWN0b3Jf bGlzdCBudGhfc2VsZWN0b3JfZW5kaW5nCi0lZGVzdHJ1Y3RvciB7IGRlbGV0ZSAkJDsgfSBzZWxl Y3Rvcl9saXN0IHNpbXBsZV9zZWxlY3Rvcl9saXN0IG50aF9zZWxlY3Rvcl9lbmRpbmcKKyV0eXBl IDxzZWxlY3Rvckxpc3Q+IHNlbGVjdG9yX2xpc3QgbmVzdGVkX3NlbGVjdG9yX2xpc3Qgc2ltcGxl X3NlbGVjdG9yX2xpc3QgbnRoX3NlbGVjdG9yX2VuZGluZworJWRlc3RydWN0b3IgeyBkZWxldGUg JCQ7IH0gc2VsZWN0b3JfbGlzdCBuZXN0ZWRfc2VsZWN0b3JfbGlzdCBzaW1wbGVfc2VsZWN0b3Jf bGlzdCBudGhfc2VsZWN0b3JfZW5kaW5nCiAKICV1bmlvbiB7IGJvb2wgYm9vbGVhbjsgfQogJXR5 cGUgPGJvb2xlYW4+IGRlY2xhcmF0aW9uIGRlY2xhcmF0aW9uX2xpc3QgZGVjbF9saXN0IHByaW9y aXR5CkBAIC0xMDkxLDYgKzEwOTEsMTUgQEAgc2VsZWN0b3JfbGlzdDoKICAgICB9CiAgICA7CiAK K2JlZm9yZV9uZXN0ZWRfc2VsZWN0b3JfbGlzdDogeyBwYXJzZXItPnN0YXJ0TmVzdGVkU2VsZWN0 b3JMaXN0KCk7IH0gOworYWZ0ZXJfbmVzdGVkX3NlbGVjdG9yX2xpc3Q6IHsgcGFyc2VyLT5lbmRO ZXN0ZWRTZWxlY3Rvckxpc3QoKTsgfSA7CisKK25lc3RlZF9zZWxlY3Rvcl9saXN0OgorICAgIGJl Zm9yZV9uZXN0ZWRfc2VsZWN0b3JfbGlzdCBzZWxlY3Rvcl9saXN0IGFmdGVyX25lc3RlZF9zZWxl Y3Rvcl9saXN0IHsKKyAgICAgICAgJCQgPSAkMjsKKyAgICB9CisgICAgOworCiBjb21wbGV4X3Nl bGVjdG9yX3dpdGhfdHJhaWxpbmdfd2hpdGVzcGFjZToKICAgICBjb21wbGV4X3NlbGVjdG9yIFdI SVRFU1BBQ0U7CiAKQEAgLTEzMDAsNyArMTMwOSw3IEBAIG50aF9zZWxlY3Rvcl9lbmRpbmc6CiAg ICAgfCBzcGFjZSAnKScgewogICAgICAgICAkJCA9IG51bGxwdHI7CiAgICAgfQotICAgIHwgc3Bh Y2UgTlRIQ0hJTERTRUxFQ1RPUlNFUEFSQVRPUiBzcGFjZSBzZWxlY3Rvcl9saXN0IG1heWJlX3Nw YWNlICcpJyB7CisgICAgfCBzcGFjZSBOVEhDSElMRFNFTEVDVE9SU0VQQVJBVE9SIHNwYWNlIG5l c3RlZF9zZWxlY3Rvcl9saXN0IG1heWJlX3NwYWNlICcpJyB7CiAgICAgICAgIGlmICgkNCkKICAg ICAgICAgICAgICQkID0gJDQ7CiAgICAgICAgIGVsc2UKQEAgLTEzMzksNyArMTM0OCw3IEBAIHBz ZXVkbzoKICAgICB9CiAKICNpZiBFTkFCTEVfQ1NTX1NFTEVDVE9SU19MRVZFTDQKLSAgICB8ICc6 JyBNQVRDSEVTRlVOQ1RJT04gbWF5YmVfc3BhY2Ugc2VsZWN0b3JfbGlzdCBtYXliZV9zcGFjZSAn KScgeworICAgIHwgJzonIE1BVENIRVNGVU5DVElPTiBtYXliZV9zcGFjZSBuZXN0ZWRfc2VsZWN0 b3JfbGlzdCBtYXliZV9zcGFjZSAnKScgewogICAgICAgICAkJCA9IG51bGxwdHI7CiAgICAgICAg IGlmICgkNCkgewogICAgICAgICAgICAgYXV0byBzZWxlY3RvciA9IHN0ZDo6bWFrZV91bmlxdWU8 Q1NTUGFyc2VyU2VsZWN0b3I+KCk7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9jc3MvQ1NT UGFyc2VyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NQYXJzZXIuY3BwCmluZGV4IGMwMGY5 Nzg5NDljNTRmMWNiY2MwNjYyMGYxNzhjNjJjZjIzZGU0YzkuLjk2MDYwMGE3YzI5ZWVjYmY3ZjA2 NGMzYmE2MTlhNjBiZmNkZGJiMzkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NQ YXJzZXIuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NQYXJzZXIuY3BwCkBAIC0zMTcs NiArMzE3LDcgQEAgQ1NTUGFyc2VyOjpDU1NQYXJzZXIoY29uc3QgQ1NTUGFyc2VyQ29udGV4dCYg Y29udGV4dCkKICAgICAsIG1faWdub3JlRXJyb3JzSW5EZWNsYXJhdGlvbihmYWxzZSkKICAgICAs IG1fZGVmYXVsdE5hbWVzcGFjZShzdGFyQXRvbSkKICAgICAsIG1fcGFyc2VkVGV4dFByZWZpeExl bmd0aCgwKQorICAgICwgbV9uZXN0ZWRTZWxlY3RvckxldmVsKDApCiAgICAgLCBtX3Byb3BlcnR5 UmFuZ2UoVUlOVF9NQVgsIFVJTlRfTUFYKQogICAgICwgbV9ydWxlU291cmNlRGF0YVJlc3VsdCgw KQogICAgICwgbV9wYXJzaW5nTW9kZShOb3JtYWxNb2RlKQpAQCAtMTIwNTIsNyArMTIwNTMsNyBA QCB2b2lkIENTU1BhcnNlcjo6bWFya1J1bGVIZWFkZXJFbmQoKQogCiB2b2lkIENTU1BhcnNlcjo6 bWFya1NlbGVjdG9yU3RhcnQoKQogewotICAgIGlmICghaXNFeHRyYWN0aW5nU291cmNlRGF0YSgp KQorICAgIGlmICghaXNFeHRyYWN0aW5nU291cmNlRGF0YSgpIHx8IG1fbmVzdGVkU2VsZWN0b3JM ZXZlbCkKICAgICAgICAgcmV0dXJuOwogICAgIEFTU0VSVCghbV9zZWxlY3RvclJhbmdlLmVuZCk7 CiAKQEAgLTEyMDYxLDcgKzEyMDYyLDcgQEAgdm9pZCBDU1NQYXJzZXI6Om1hcmtTZWxlY3RvclN0 YXJ0KCkKIAogdm9pZCBDU1NQYXJzZXI6Om1hcmtTZWxlY3RvckVuZCgpCiB7Ci0gICAgaWYgKCFp c0V4dHJhY3RpbmdTb3VyY2VEYXRhKCkpCisgICAgaWYgKCFpc0V4dHJhY3RpbmdTb3VyY2VEYXRh KCkgfHwgbV9uZXN0ZWRTZWxlY3RvckxldmVsKQogICAgICAgICByZXR1cm47CiAgICAgQVNTRVJU KCFtX3NlbGVjdG9yUmFuZ2UuZW5kKTsKICAgICBBU1NFUlQobV9jdXJyZW50UnVsZURhdGFTdGFj ay0+c2l6ZSgpKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NQYXJzZXIuaCBi L1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NQYXJzZXIuaAppbmRleCA1OWUxZjZlMTQxN2Y0OGQ4ZTEy Y2RmYjE1YTk3MzQ4ZmJhNmFlOWQzLi44MTY3ZDgyZjc1ODFlZjBjMmFmZjViNGEyNWRhODg3NWU1 ZDllZmUxIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9jc3MvQ1NTUGFyc2VyLmgKKysrIGIv U291cmNlL1dlYkNvcmUvY3NzL0NTU1BhcnNlci5oCkBAIC0zNzUsNiArMzc1LDcgQEAgcHVibGlj OgogCiAgICAgLy8gdG9rZW5pemVyIG1ldGhvZHMgYW5kIGRhdGEKICAgICBzaXplX3QgbV9wYXJz ZWRUZXh0UHJlZml4TGVuZ3RoOworICAgIHVuc2lnbmVkIG1fbmVzdGVkU2VsZWN0b3JMZXZlbDsK ICAgICBTb3VyY2VSYW5nZSBtX3NlbGVjdG9yUmFuZ2U7CiAgICAgU291cmNlUmFuZ2UgbV9wcm9w ZXJ0eVJhbmdlOwogICAgIHN0ZDo6dW5pcXVlX3B0cjxSdWxlU291cmNlRGF0YUxpc3Q+IG1fY3Vy cmVudFJ1bGVEYXRhU3RhY2s7CkBAIC0zODQsOCArMzg1LDEyIEBAIHB1YmxpYzoKICAgICB2b2lk IGZpeFVucGFyc2VkUHJvcGVydHlSYW5nZXMoQ1NTUnVsZVNvdXJjZURhdGEqKTsKICAgICB2b2lk IG1hcmtSdWxlSGVhZGVyU3RhcnQoQ1NTUnVsZVNvdXJjZURhdGE6OlR5cGUpOwogICAgIHZvaWQg bWFya1J1bGVIZWFkZXJFbmQoKTsKKworICAgIHZvaWQgc3RhcnROZXN0ZWRTZWxlY3Rvckxpc3Qo KSB7ICsrbV9uZXN0ZWRTZWxlY3RvckxldmVsOyB9CisgICAgdm9pZCBlbmROZXN0ZWRTZWxlY3Rv ckxpc3QoKSB7IC0tbV9uZXN0ZWRTZWxlY3RvckxldmVsOyB9CiAgICAgdm9pZCBtYXJrU2VsZWN0 b3JTdGFydCgpOwogICAgIHZvaWQgbWFya1NlbGVjdG9yRW5kKCk7CisKICAgICB2b2lkIG1hcmtS dWxlQm9keVN0YXJ0KCk7CiAgICAgdm9pZCBtYXJrUnVsZUJvZHlFbmQoKTsKICAgICB2b2lkIG1h cmtQcm9wZXJ0eVN0YXJ0KCk7Cg==