Bug 205035

Summary: Web Inspector: REGRESSION(r251038): Elements: Computed: implicit shorthands are not shown when "Prefer Shorthands" is enabled
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Devin Rousso <drousso>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, drousso, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 200554    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Description Nikita Vasilyev 2019-12-09 16:23:37 PST
Inspect:

    <body style="font-family: -webkit-system-font, sans-serif; font-size: 75%;">

Computed panel should include `font-family` and `font-size` values.

Instead, it only shows:

    color: rgb(0, 0, 0);
    display: block;
    height: 90px;
    width: 411px;
Comment 1 Radar WebKit Bug Importer 2019-12-09 16:24:07 PST
<rdar://problem/57773470>
Comment 2 Nikita Vasilyev 2019-12-09 16:27:47 PST
Reduction: https://nv.github.io/webkit-inspector-bugs/205035_computed-shortcuts-bug/index.html

This regressed in Bug 200554 Web Inspector: Elements: Computed: show shorthand properties in addition to longhand ones
Comment 3 Devin Rousso 2019-12-09 17:46:36 PST
Created attachment 385218 [details]
Patch
Comment 4 Brian Burg 2019-12-10 12:12:02 PST
Comment on attachment 385218 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=385218&action=review

r=me

> Source/WebInspectorUI/ChangeLog:17
> +        Drive-by: filter the list of properties to render before sorting them for performance.

Does this affect the display order? Is it already being sorted?

> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.js:170
> +                if (!(this._showsShorthandsInsteadOfLonghands && property.isShorthand && hasNonImplicitLonghand(property)))

I wish it were possible to fold this into the guard below, but alas, it early exits too early otherwise.
Comment 5 Devin Rousso 2019-12-10 14:05:52 PST
Comment on attachment 385218 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=385218&action=review

>> Source/WebInspectorUI/ChangeLog:17
>> +        Drive-by: filter the list of properties to render before sorting them for performance.
> 
> Does this affect the display order? Is it already being sorted?

No, it was already sorted.  What I mean by saying this is rather than sort-then-filter, we filter-then-sort so we don't have to sort as many items :P

>> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.js:170
>> +                if (!(this._showsShorthandsInsteadOfLonghands && property.isShorthand && hasNonImplicitLonghand(property)))
> 
> I wish it were possible to fold this into the guard below, but alas, it early exits too early otherwise.

Indeed :(
Comment 6 WebKit Commit Bot 2019-12-10 15:00:55 PST
The commit-queue encountered the following flaky tests while processing attachment 385218 [details]:

fetch/fetch-worker-crash.html bug 187257 (author: youennf@gmail.com)
The commit-queue is continuing to process your patch.
Comment 7 WebKit Commit Bot 2019-12-10 15:01:54 PST
Comment on attachment 385218 [details]
Patch

Clearing flags on attachment: 385218

Committed r253348: <https://trac.webkit.org/changeset/253348>
Comment 8 WebKit Commit Bot 2019-12-10 15:01:56 PST
All reviewed patches have been landed.  Closing bug.