NEW 139750
Web Inspector: show user agent style properties as shorthand if it looks better than longhand 
https://bugs.webkit.org/show_bug.cgi?id=139750
Summary Web Inspector: show user agent style properties as shorthand if it looks bett...
Joseph Pecoraro
Reported 2014-12-17 15:04:33 PST
* SUMMARY <body> shows unexpected UserAgent margin styles (one non-implicit, others implicit). I would expect them all to be the same, the reason User Agent stylesheet says "margin: 8px". * STEPS TO REPRODUCE 1. Inspect <body> in data:text/html,<body> 2. Show Rules in Styles details sidebar => "margin" values shown in unexpected way * ACTUAL RESULTS: display: block; /* non-implicit */ margin-top: 8px; /* non-implicit */ margin-right: 8px; /* implicit */ margin-bottom: 8px; /* implicit */ margin-left: 8px; /* implicit */ * EXPECTED RESULTS: display: block; /* non-implicit */ margin: 8px; /* non-implicit */
Attachments
Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2014-12-17 15:04:49 PST
<rdar://problem/19285473>
Joseph Pecoraro
Comment 2 2014-12-17 15:19:46 PST
Looks like an issue with the backend reply for CSS.getMatchedStylesForNode dealing with shorthanded entries. For "margin: 8px" it sends some of the long-hands as implicit. frontend: { "method": "CSS.getMatchedStylesForNode", "params": { "nodeId": 8, "includePseudo": true, "includeInherited": true }, "id": 52 } backend: { "result": { "matchedCSSRules": [{ "rule": { "selectorList": { "selectors": [{ "text": "body", "specificity": [0, 0, 1] }], "text": "body" }, "sourceLine": 0, "origin": "user-agent", "style": { "cssProperties": [{ "name": "display", "value": "block" }, { "name": "margin-top", "value": "8px" }, { "name": "margin-right", "value": "8px", "implicit": true }, { "name": "margin-bottom", "value": "8px", "implicit": true }, { "name": "margin-left", "value": "8px", "implicit": true }], "shorthandEntries": [{ "name": "margin", "value": "8px" }], "styleId": { "styleSheetId": "4", "ordinal": 2 }, "width": "", "height": "" } }, "matchingSelectors": [0] }], ... }, "id": 52 }
Joseph Pecoraro
Comment 3 2015-01-06 15:55:59 PST
This is especially ugly for something like <textarea>!
Timothy Hatcher
Comment 4 2015-01-06 16:04:33 PST
I agree we should just show margin: 8px here. We don't need to show the long-hands. The backend reporting "implicit" is correct here. The only value specified specifically is margin-top. The rest are implied from that one value. For example: margin: 8px 4px; would yield margin-top and margin-right with 8px and 4px, and the margin-bottom and margin-left would be implicit. What use to do was show the shorthand, with the long-hands expandable under it. I'm not sure we should do that again, and just show the shorthand by itself.
Timothy Hatcher
Comment 5 2015-01-06 16:11:33 PST
We don't have the original source for the user agent rules, which causes us to synthesize the source. We just need to be smarter and the long-hands that are covered by the shorthandEntries list when synthesizing the source.
Joseph Pecoraro
Comment 6 2015-01-06 16:32:21 PST
(In reply to comment #4) > The backend reporting "implicit" is correct here. The only value specified > specifically is margin-top. The rest are implied from that one value. I don't think that is true. The User-Agent styles are: /* Source/WebCore/css/html.css */ body { display: block; margin: 8px } margin-top is no more specified then margin-left or the others. In my opinion they should either all be implicit or not. I guess I don't understand why WebKit says otherwise: var list = document.defaultView.getMatchedCSSRules(document.body); var rule = list[2]; var decl = rule.style; rule.cssText; // "body { font-family: Verdana, Arial, Helvetica; font-size: 12px; padding: 0px 5px; margin: 0px; }" decl.isPropertyImplicit("margin"); // false decl.isPropertyImplicit("margin-top") // false decl.isPropertyImplicit("margin-left") // true decl.isPropertyImplicit("margin-right") // true decl.isPropertyImplicit("margin-bottom") // true I guess I don't actually know why something is considered implicit, or there is a bug. > For example: margin: 8px 4px; would yield margin-top and margin-right with > 8px and 4px, and the margin-bottom and margin-left would be implicit. > > What use to do was show the shorthand, with the long-hands expandable under > it. I'm not sure we should do that again, and just show the shorthand by > itself. I agree with just showing the shorthand. -- Here is an interesting case. Is there ever a case with ambiguity like this? div { margin: 5px; margin-right: 10px; } If there is a case like this in non-author styles, I think we should be unambiguous, and probably list everything.
Joseph Pecoraro
Comment 7 2015-01-06 16:49:31 PST
After chatting with Timothy Hatcher on IRC. Implicit properties are ones not present in the shorthand. e.g. in "margin: 5px" only "top" is present and the rest are implicit, whereas in "margin: 5px 5px 5px 5px" they are all present. In that case, there are a few changes here. - I don't think showing transparent "implicit" values is useful data in the "Rules" section of the Styles sidebar. We should drop that. - We should show the best user visible string to the user - if a longhand value is overridden by author styles, show all longhand values to show exactly what is overridden - otherwise, it is safe to show the shorthand, but perhaps only show shorthands for simple properties like "margin/padding", not "font/background" which are more complicated for developers to remember exactly what it set See related: <https://webkit.org/b/140154> Web Inspector: Expected UserAgent styles to be crossed-out if overridden
Joseph Pecoraro
Comment 8 2015-01-06 18:12:35 PST
Addressing transparency with: <https://webkit.org/b/140161> Web Inspector: Do not style implicit CSS properties in the Style Rules section Leaving this open to handle the actual logic for shorthand vs longhand. Also, it appears we are over-loading the term "implicit", which makes this even more confusing, but that is also a separate issue.
Note You need to log in before you can comment on or make changes to this bug.