Bug 167075 - Web Inspector: split console should be allowed for Network and Timeline tabs when Inspector is not docked
Summary: Web Inspector: split console should be allowed for Network and Timeline tabs ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-01-15 14:23 PST by BJ Burg
Modified: 2017-01-18 21:11 PST (History)
4 users (show)

See Also:


Attachments
Patch (5.10 KB, patch)
2017-01-15 21:24 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (5.22 KB, patch)
2017-01-16 21:13 PST, 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 BJ Burg 2017-01-15 14:23:11 PST
The only problematic case is when docked to bottom, we have basically no room at all. I am pretty sure that this is a getter or method call, so this can be done dynamically. (In that case I don't think we should ever show split console while docked to bottom).
Comment 1 Radar WebKit Bug Importer 2017-01-15 14:23:21 PST
<rdar://problem/30033611>
Comment 2 Devin Rousso 2017-01-15 21:24:53 PST
Created attachment 298937 [details]
Patch
Comment 3 Timothy Hatcher 2017-01-16 18:54:13 PST
Comment on attachment 298937 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/ContentView.js:313
> -        return true;
> +        return WebInspector.dockedConfigurationSupportsSplitContentBrowser();

Maybe if this is the default then the subclass methods that do the same thing can just be removed.

> Source/WebInspectorUI/UserInterface/Views/NetworkTabContentView.js:64
> -        return false;
> +        return WebInspector.dockedConfigurationSupportsSplitContentBrowser();

This still poses a problem. Since Network has real sidebar, if the split console is shown, the sidebar will show things that don't line up with the DataGrid in the content view. A comment to that effect should be added to explain this. Give it a try with a lot of network requests and you will see what I mean.

> Source/WebInspectorUI/UserInterface/Views/TimelineTabContentView.js:314
> -        return false;
> +        return WebInspector.dockedConfigurationSupportsSplitContentBrowser();

Timeline use to be the same way, but we don't have a real sidebar anymore so it should be fine.
Comment 4 Devin Rousso 2017-01-16 21:05:15 PST
Comment on attachment 298937 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/ContentView.js:313
>> +        return WebInspector.dockedConfigurationSupportsSplitContentBrowser();
> 
> Maybe if this is the default then the subclass methods that do the same thing can just be removed.

Yup.  100% correct.  No idea why I didn't do that...

>> Source/WebInspectorUI/UserInterface/Views/NetworkTabContentView.js:64
>> +        return WebInspector.dockedConfigurationSupportsSplitContentBrowser();
> 
> This still poses a problem. Since Network has real sidebar, if the split console is shown, the sidebar will show things that don't line up with the DataGrid in the content view. A comment to that effect should be added to explain this. Give it a try with a lot of network requests and you will see what I mean.

Oh snap that's a really good point.  I'll revert this.
Comment 5 Devin Rousso 2017-01-16 21:13:11 PST
Created attachment 299016 [details]
Patch
Comment 6 WebKit Commit Bot 2017-01-18 21:11:00 PST
Comment on attachment 299016 [details]
Patch

Clearing flags on attachment: 299016

Committed r210911: <http://trac.webkit.org/changeset/210911>
Comment 7 WebKit Commit Bot 2017-01-18 21:11:04 PST
All reviewed patches have been landed.  Closing bug.