Bug 221246

Summary: Web Inspector: remove experimental setting and enable Layout sidebar
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web InspectorAssignee: Nikita Vasilyev <nvasilyev>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, ews-watchlist, hi, inspector-bugzilla-changes, nvasilyev, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 223209    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description BJ Burg 2021-02-01 22:57:28 PST 
Placeholder bug for later.
Comment 1 BJ Burg 2021-02-01 23:00:56 PST 
As part of this, the engineering-only context menu items for showing/hiding grid overlays should be removed. At that point they will no longer be necessary.
Comment 2 Radar WebKit Bug Importer 2021-02-08 22:58:12 PST 
<rdar://problem/74130753>
Comment 3 Nikita Vasilyev 2021-03-15 12:37:45 PDT 
(In reply to BJ Burg from comment #1)
> As part of this, the engineering-only context menu items for showing/hiding
> grid overlays should be removed. At that point they will no longer be
> necessary.

This was removed in Web Inspector: CSS Grid Inspector clean-up
https://trac.webkit.org/changeset/274229/webkit
Comment 4 Nikita Vasilyev 2021-03-15 12:41:13 PDT Comment hidden (obsolete)
Comment 5 Nikita Vasilyev 2021-03-15 12:47:54 PDT 
Created attachment 423223 [details]
Patch
Comment 6 Devin Rousso 2021-03-15 12:50:35 PDT 
Comment on attachment 423223 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/ElementsTabContentView.js:33
> +            WI.LayoutDetailsSidebarPanel,

Shouldn't this only be added if the backend has `CSS.nodeLayoutContextTypeChanged` (and/or any of the other new protocol things)?
Comment 7 Nikita Vasilyev 2021-03-15 16:59:08 PDT 
Created attachment 423264 [details]
Patch
Comment 8 Nikita Vasilyev 2021-03-15 17:00:45 PDT 
Comment on attachment 423264 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/ElementsTabContentView.js:36
> +        // COMPATIBILITY (iOS 14.5): `DOM.showGridOverlay` did not exist yet.
> +        if (InspectorBackend.hasCommand("DOM.showGridOverlay"))

I don't know if should check for anything else here. Other commands were added around the same time, and it looks like we usually check for only one command elsewhere in our codebase.
Comment 9 BJ Burg 2021-03-15 22:44:20 PDT 
Comment on attachment 423264 [details]
Patch

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

r=me

>> Source/WebInspectorUI/UserInterface/Views/ElementsTabContentView.js:36
>> +        if (InspectorBackend.hasCommand("DOM.showGridOverlay"))
> 
> I don't know if should check for anything else here. Other commands were added around the same time, and it looks like we usually check for only one command elsewhere in our codebase.

This is fine.
Comment 10 EWS 2021-03-15 22:55:24 PDT 
Committed r274466: <https://commits.webkit.org/r274466>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423264 [details].