Bug 222364 – Web Inspector: List of grid nodes is incomplete in Layout sidebar panel

• Component: Web Inspector
• Status: RESOLVED FIXED

[Razvan Caliman]

The list of grid nodes is incomplete when opening Web Inspector and the Layout sidebar panel is selected by default.

Steps to reproduce:

- Open Web Inspector and select the Layout sidebar panel of the Elements Tab
- Close Web Inspector 
- Navigate to https://stripe.com
- Open Web Inspector again

Expected result:
 
A long list of grid nodes (100+) in the CSS Grid section of the Layout sidebar panel.

Actual result:

Either no nodes or a short list of nodes matching any grid nodes already expanded in the DOM tree.

[Razvan Caliman]

Workaround: 

Select another sidebar panel (ex: Computed), then select Layout again.
This populates the list with all the grid nodes.

[Razvan Caliman]

Created attachment 421416 [details]
WIP

quick hack that works

[Razvan Caliman]

Seems that the backend doesn't push nodes to the frontend when first setting `WI.cssManager.layoutContextTypeChangedMode = WI.CSSManager.LayoutContextTypeChangedMode.All` in `WI.LayoutDetailsSidebarPanel.attached()`

When that is set, `WI.DOMNode` instances for grid nodes should be created automatically  and `WI.DOMNode.Event.LayoutContextTypeChanged` events should fire (see last line of `WI.DOMNode` constructor). But none do. The workaround of switching between panels unblocks this and events come through as expected.

The only thing that jumps out is that `WI.LayoutDetailsSidebarPanel.detached()` changes the mode to "Observed". 🤔 

Strangely, toggling the mode in `WI.LayoutDetailsSidebarPanel.attached()` solves the bug:

```diff
+ WI.cssManager.layoutContextTypeChangedMode = WI.CSSManager.LayoutContextTypeChangedMode.Observed;
  WI.cssManager.layoutContextTypeChangedMode = WI.CSSManager.LayoutContextTypeChangedMode.All;
```

See [WIP](https://bugs.webkit.org/attachment.cgi?id=421416)

This is a hack.

@Patrick Angle, is there any reason why nodes wouldn't get pushed to the frontend when first setting the mode to "All"?

[Radar WebKit Bug Importer]

<rdar://problem/74700960>

[Patrick Angle]

(In reply to Razvan Caliman from comment #3)
> @Patrick Angle, is there any reason why nodes wouldn't get pushed to the
> frontend when first setting the mode to "All"?

I think I see my mistake. I'm not honoring the current layout state when enabling the `WebCore::InspectorCSSAgent` because the assumption is that it will be in the `Observed` state, but that isn't the case here because it was already in the `All` state when we enable it. Two solutions here, but I think the better one is to add `m_layoutContextTypeChangedMode = Protocol::CSS::LayoutContextTypeChangedMode::Observed;` to `WebCore::InspectorCSSAgent::reset`. This would be consistent with other properties that get reset when the agent is reset (which also happens when the agent is disabled).

[Razvan Caliman]

Created attachment 421538 [details]
Patch

[EWS Watchlist]

This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.

[Patrick Angle]

Comment on attachment 421538 [details]
Patch

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

> Source/JavaScriptCore/inspector/protocol/CSS.json:266
> +            "description": "The mode for how layout context type changes are handled (default: observed). <code>Observed</code> limits handling to those nodes already known to the frontend by other means (generally, this means the node is a visible item in the Elements tab). <code>All</code> informs the frontend of all layout context type changes and all nodes with a known layout context are sent to the frontend."

NIT: `(default: <code>Observed</code>)`

[Razvan Caliman]

Created attachment 421548 [details]
Patch

Address nit

[Devin Rousso]

Comment on attachment 421548 [details]
Patch

r=me assuming EWS is green :)

[Blaze Burg]

Comment on attachment 421548 [details]
Patch

mac-wk2 passed, setting cq+

[EWS]

Committed r273502: <https://commits.webkit.org/r273502>

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