Bug 161966 - Web Inspector: The inspector sidebar has weird sub-column sizing issues
Summary: Web Inspector: The inspector sidebar has weird sub-column sizing issues
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: Safari Technology Preview
Hardware: All OS X 10.11
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-09-14 08:56 PDT by David Gasperoni
Modified: 2017-03-27 16:31 PDT (History)
6 users (show)

See Also:


Attachments
The inspector sidebar has weird multi sub-column sizing issues (1.65 MB, video/mp4)
2016-09-14 08:56 PDT, David Gasperoni
no flags Details
Patch (11.30 KB, patch)
2017-03-23 18:54 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
[Video] Grid column resize bug (1.99 MB, video/mp4)
2017-03-23 19:51 PDT, Matt Baker
no flags Details
Patch (13.35 KB, patch)
2017-03-27 15:01 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Gasperoni 2016-09-14 08:56:27 PDT
Created attachment 288822 [details]
The inspector sidebar has weird multi sub-column sizing issues

In the Network panel (maybe others?) the sidebar suffers from a weird sub-column sizing issue. I cannot describe it, I made a 50 seconds screencast that I attached.
Comment 1 Radar WebKit Bug Importer 2016-09-14 08:57:27 PDT
<rdar://problem/28300919>
Comment 2 David Gasperoni 2017-03-23 11:57:20 PDT
Still an issue in Safari Technology Preview release 26.
Comment 3 Devin Rousso 2017-03-23 18:54:04 PDT
Created attachment 305257 [details]
Patch
Comment 4 Matt Baker 2017-03-23 19:51:17 PDT
Created attachment 305260 [details]
[Video] Grid column resize bug

Grid column resize elements are highlighted in green, to illustrate the root of the problem.

Column resizers need to be repositioned after the parent sidebar panel's width is changed.
Comment 5 Joseph Pecoraro 2017-03-24 00:57:41 PDT
Comment on attachment 305257 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:14
> +        * UserInterface/Views/ComputedStyleDetailsPanel.js:

At a glance this all looks good. Great job finding a lot of the sidebar datagrids. I'd expect one more: ProbeDetailsSidebarPanel -> ProbeSetDetailsSection -> its DataGrid (_dataGrid). Let me know if you need instructions for testing probes.
Comment 6 Matt Baker 2017-03-24 10:19:05 PDT
I think we'll want a different solution in the long term. View.sizeDidChange was intended to be called internally during layout. This could be changed, but that was the original intent. Currently sizeDidChange is called by the View base class when LayoutReason.Resize is passed to needsLayout/updateLayout. We may want to invert this pattern, and instead tell a view that its size has changed, and let it determine whether to cause a layout on itself or its subtree.

View would need to have a flag indicating that it needs to perform a layout whenever it (or its parent) is resized. Like the dirty flag on View, this flag would propagate up the parent path in the view hierarchy, but unlike the dirty flag it would be set once when the view is created, and not change.

I'm not suggesting you start over with this patch, but this should be revisited in the future.
Comment 7 Devin Rousso 2017-03-27 15:01:43 PDT
Created attachment 305515 [details]
Patch
Comment 8 Joseph Pecoraro 2017-03-27 15:19:02 PDT
Comment on attachment 305515 [details]
Patch

r=me
Comment 9 WebKit Commit Bot 2017-03-27 16:31:05 PDT
Comment on attachment 305515 [details]
Patch

Clearing flags on attachment: 305515

Committed r214439: <http://trac.webkit.org/changeset/214439>
Comment 10 WebKit Commit Bot 2017-03-27 16:31:08 PDT
All reviewed patches have been landed.  Closing bug.