Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Web Inspector: Resizing Debugger Resource/Scope Chain column doesn't …
…survive the change of tab

https://bugs.webkit.org/show_bug.cgi?id=256691
rdar://109253229

Reviewed by Devin Rousso.

The width of the details sidebar for the Sources tab resets to its minimum width when navigating
to a tab without sidebar panels (Console, Network) or to a tab with a collapsed details sidebar (Graphics, Storage).

Navigating between tabs with visible details sidebar (Sources, Elements) doesn't reset the sidebar width.

TL;DR:

Adding and removing sidebar panels triggers `SingleSidebar._recalculateWidth(newWidth = this.width)`
without passing `newWidth` which therefore uses the default `this.width`.

While the sidebar is collapsed, `this.width` is zero (the sidebar DOM element's `offsetWidth`).

These calls clobber any previously set `width` on the sidebar with zero.
But there's a clamp function in `SingleSidebar._recalculateWidth()` which picks
the `SingleSidebar.minimumWidth` for the sidebar. This resets the sidebar width.

Detailed explantion:

--- Symptom 1

When a tab without visible sidebar panels is selected, the details sidebar gets collapsed:
- [TabBrowser._showDetailsSidebarPanelsForTabContentView():L471-L473](https://github.com/WebKit/WebKit/blob/fc2cbdef688b134951ca32f386632379ea8cf49f/Source/WebInspectorUI/UserInterface/Views/TabBrowser.js#L471-L473)
- [ContentBrowserTabContentView.showDetailsSidebarPanels():L195-L196](https://github.com/WebKit/WebKit/blob/fc2cbdef688b134951ca32f386632379ea8cf49f/Source/WebInspectorUI/UserInterface/Views/ContentBrowserTabContentView.js#L195-L196)

--- Symptom 2

The `width` getter for `SingleSidebar` returns it from the corresponding DOM node:
```
get width()
{
    return this.element.offsetWidth;
}
```

The `width` setter calls `SingleSidebar._recalculateWidth(newWidth = this.width)`:

[SingleSidebar._recalculateWidth():L175-L192](https://github.com/WebKit/WebKit/blob/fc2cbdef688b134951ca32f386632379ea8cf49f/Source/WebInspectorUI/UserInterface/Views/SingleSidebar.js#L175C5-L192)

That default function parameter, `newWidth = this.width`, is the source of the issue.

--- Explanation of issue:

When the sidebar is collapsed, the layout step is skipped in `SingleSidebar._recalculateWidth()`:

```
if (this.collapsed)
    return;

if (this._navigationBar)
    this._navigationBar.needsLayout(WI.View.LayoutReason.Resize);

if (this.selectedSidebarPanel)
    this.selectedSidebarPanel.needsLayout(WI.View.LayoutReason.Resize);
```

That means that, even though, there's a non-zero `newWidth` on the element,
the `.collapsed` CSS class on the sidebar makes its `offsetWidth` equal to zero.

When sidebar panels are added in `ContentBrowserTabContentView.showDetailsSidebarPanels()`,
they trigger layout via this call chain:

`SingleSidebar.insertSidebarPanel()`
-> `SingleSidebar.didInsertSidebarPanel()`
-> `SingleSidebar._recalculateWidth(newWidth = this.width)`

But in this call there is no value for `newWidth`, so it uses the default function paramter
which calls the `SingleSidebar.width` getter that returns zero (the DOM node's actual `offsetWidth`).

Luckily, there's also an immutable minimum width on the sidebar styles set by `SingleSidebar._recalculateWidth():

```
    this.element.style.width = `${newWidth}px`; // this is 0px
    this.element.style.minWidth = `${this.minimumWidth}px`; // this is 250px
```

This allows the sidebar content to be shown but at a different size than the one set when resizing the sidebar.

--- Solution:

Prevent calling `SingleSidebar._recalculateWidth()` when sidebar panels are inserted or removed
while the sidebar is collapsed. This avoids overwriting the element's inline style `width` to zero.

* Source/WebInspectorUI/UserInterface/Views/SingleSidebar.js:
(WI.SingleSidebar.prototype.didInsertSidebarPanel):
(WI.SingleSidebar.prototype._recalculateWidth):

Canonical link: https://commits.webkit.org/265152@main
  • Loading branch information
rcaliman-apple committed Jun 14, 2023
1 parent e553d77 commit 8303e81
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions Source/WebInspectorUI/UserInterface/Views/SingleSidebar.js
Expand Up @@ -89,6 +89,9 @@ WI.SingleSidebar = class SingleSidebar extends WI.Sidebar
console.assert(sidebarPanel.navigationItem);
this._navigationBar.insertNavigationItem(sidebarPanel.navigationItem, index);

if (this.collapsed)
return;

this._recalculateWidth();
}

Expand All @@ -100,6 +103,9 @@ WI.SingleSidebar = class SingleSidebar extends WI.Sidebar
console.assert(sidebarPanel.navigationItem);
this._navigationBar.removeNavigationItem(sidebarPanel.navigationItem);

if (this.collapsed)
return;

this._recalculateWidth();
}

Expand Down Expand Up @@ -174,6 +180,8 @@ WI.SingleSidebar = class SingleSidebar extends WI.Sidebar

_recalculateWidth(newWidth = this.width)
{
console.assert(newWidth);

// Need to add 1 because of the 1px border-inline-start or border-inline-end.
newWidth = Math.ceil(Number.constrain(newWidth, this.minimumWidth + 1, this.maximumWidth));
this.element.style.width = `${newWidth}px`;
Expand Down

0 comments on commit 8303e81

Please sign in to comment.