215681 2020-08-19 20:03:41 -0700 Web Inspector: Elements: Styles: don't show swatches for properties that aren't used/apply 2020-08-24 10:24:57 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 215680 1 hi hi bburg hi inspector-bugzilla-changes nvasilyev webkit-bug-importer oldest_to_newest 1681588 0 hi 2020-08-19 20:03:41 -0700 a common pattern in CSS is to have two of the same property next to each other, the earlier intended as a fallback for the latter ``` color: black; color: var(--color); ``` showing a swatch in front of the former property isn't all that useful as it's not used/applied and just adds to the confusion of what is used/applied (especially after bug 215680) 1681591 1 406903 hi 2020-08-19 20:05:28 -0700 Created attachment 406903 Patch 1681611 2 nvasilyev 2020-08-19 21:05:09 -0700 I'm a little bit on the fence about this. Leaning towards that the benefits of this patch outweigh the downsides. On one hand, the strikethrough doesn't go over the swatches and it looks bad. From the visual design perspective, this patch is an improvement. On the other hand, I find it useful to see what colors are overridden. 1681763 3 hi 2020-08-20 12:20:35 -0700 (In reply to Nikita Vasilyev from comment #2) > On one hand, the strikethrough doesn't go over the swatches and it looks bad. From the visual design perspective, this patch is an improvement. Agreed. > On the other hand, I find it useful to see what colors are overridden. Really? I've never really found this to be something I cared for. Not to mention, in my experience the thing that's overridden is often a raw color (i.e. not a variable), meaning that I can just read it to see what color is used. 1682359 4 406903 bburg 2020-08-24 10:00:34 -0700 Comment on attachment 406903 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406903&action=review r=me with a question. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:466 > + if (this._property.enabled && !this._property.overridden && this._property.valid && !this._property.hasOtherVendorNameOrKeyword()) Do we support any hasOtherVendorNameOrKeyword properties that do not have an unprefixed counterpart? This seems fine, unless it means some obscure --webkit- properties no longer get swatches. 1682367 5 webkit-bug-importer 2020-08-24 10:11:13 -0700 <rdar://problem/67685687> 1682379 6 406903 hi 2020-08-24 10:22:58 -0700 Comment on attachment 406903 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406903&action=review >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:466 >> + if (this._property.enabled && !this._property.overridden && this._property.valid && !this._property.hasOtherVendorNameOrKeyword()) > > Do we support any hasOtherVendorNameOrKeyword properties that do not have an unprefixed counterpart? This seems fine, unless it means some obscure --webkit- properties no longer get swatches. IIRC `hasOtherVendorNameOrKeyword` checks to see whether the name/value starts with `-moz-`/`-ms-`/`-o-`, so it shouldn't ever affect anything starting with `-webkit`. 1682380 7 ews-feeder 2020-08-24 10:24:56 -0700 Committed r266070: <https://trac.webkit.org/changeset/266070> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406903. 406903 2020-08-19 20:05:28 -0700 2020-08-24 10:24:57 -0700 Patch 215681.diff text/plain 1549 hi ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IGQ1NTNiOTk0M2JkMi4uYmExYWMzOGIzYWVhIDEw MDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9X ZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMyBAQAorMjAyMC0wOC0xOSAgRGV2 aW4gUm91c3NvICA8ZHJvdXNzb0BhcHBsZS5jb20+CisKKyAgICAgICAgV2ViIEluc3BlY3Rvcjog RWxlbWVudHM6IFN0eWxlczogZG9uJ3Qgc2hvdyBzd2F0Y2hlcyBmb3IgcHJvcGVydGllcyB0aGF0 IGFyZW4ndCB1c2VkL2FwcGx5CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0yMTU2ODEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICAqIFVzZXJJbnRlcmZhY2UvVmlld3MvU3ByZWFkc2hlZXRTdHlsZVByb3BlcnR5 LmpzOgorICAgICAgICAoV0kuU3ByZWFkc2hlZXRTdHlsZVByb3BlcnR5LnByb3RvdHlwZS5fcmVu ZGVyVmFsdWUpOgorCiAyMDIwLTA4LTE0ICBEZXZpbiBSb3Vzc28gIDxkcm91c3NvQGFwcGxlLmNv bT4KIAogICAgICAgICBXZWIgSW5zcGVjdG9yOiBBc3NlcnRpb24gRmFpbGVkOiBBcHBlbmRlZCBh IHN0cmluZyB3aXRoIG5ld2xpbmVzLiBUaGlzIGJyZWFrcyB0aGUgc291cmNlIG1hcC4KZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NwcmVhZHNo ZWV0U3R5bGVQcm9wZXJ0eS5qcyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNl L1ZpZXdzL1NwcmVhZHNoZWV0U3R5bGVQcm9wZXJ0eS5qcwppbmRleCBjZjdkNTc3MDg2NDEuLmUy NGU2MWI4Nzc1MSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZh Y2UvVmlld3MvU3ByZWFkc2hlZXRTdHlsZVByb3BlcnR5LmpzCisrKyBiL1NvdXJjZS9XZWJJbnNw ZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NwcmVhZHNoZWV0U3R5bGVQcm9wZXJ0eS5qcwpA QCAtNDYzLDcgKzQ2Myw3IEBAIFdJLlNwcmVhZHNoZWV0U3R5bGVQcm9wZXJ0eSA9IGNsYXNzIFNw cmVhZHNoZWV0U3R5bGVQcm9wZXJ0eSBleHRlbmRzIFdJLk9iamVjdAogICAgICAgICBjb25zdCBt YXhWYWx1ZUxlbmd0aCA9IDE1MDsKICAgICAgICAgbGV0IHRva2VucyA9IFdJLnRva2VuaXplQ1NT VmFsdWUodmFsdWUpOwogCi0gICAgICAgIGlmICh0aGlzLl9wcm9wZXJ0eS5lbmFibGVkKQorICAg ICAgICBpZiAodGhpcy5fcHJvcGVydHkuZW5hYmxlZCAmJiAhdGhpcy5fcHJvcGVydHkub3ZlcnJp ZGRlbiAmJiB0aGlzLl9wcm9wZXJ0eS52YWxpZCAmJiAhdGhpcy5fcHJvcGVydHkuaGFzT3RoZXJW ZW5kb3JOYW1lT3JLZXl3b3JkKCkpCiAgICAgICAgICAgICB0b2tlbnMgPSB0aGlzLl9yZXBsYWNl U3BlY2lhbFRva2Vucyh0b2tlbnMpOwogCiAgICAgICAgIHRva2VucyA9IHRva2Vucy5tYXAoKHRv a2VuKSA9PiB7Cg==