Bug 138518

Summary: Web Inspector: Layer summary should be bottom sticky
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: graouts, joepeck, simon.fraser, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix timothy: review+

Description Joseph Pecoraro 2014-11-07 14:27:21 PST 
* SUMMARY
The summary at the bottom of the layers list should be bottom sticky, so it can always be seen for long layer lists.

* STEPS TO REPRODUCE
1. Inspect apple.com
2. Show Layers Details Sidebar
3. Click a link to open a video
  => layers show up, summary jumps around

* NOTES
The sidebar contents should slide under the summary pinned to the bottom, and yet still be scrollable so everything in the sidebar can be reached.
Comment 1 Radar WebKit Bug Importer 2014-11-07 14:27:47 PST 
<rdar://problem/18913472>
Comment 2 Joseph Pecoraro 2014-11-07 14:28:52 PST 
Better radar: <rdar://problem/18910924>
Comment 3 Joseph Pecoraro 2014-11-07 14:52:38 PST 
Created attachment 241208 [details]
[PATCH] Proposed Fix

I think I saw one regression from this.

* STEPS TO REPRODUCE
1. Inspect apple.com
2. Show Styles for <body>
3. Show Computed section, with "Show All" enabled
4. Scroll way down in the sidebar
5. Uncheck "Show All"
  => doesn't scroll the new content into view appropriately

But right now I think this is a WebCore issue.
Comment 4 Joseph Pecoraro 2014-11-07 14:55:38 PST 
Comment on attachment 241208 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/WebInspectorUI.vcxproj/WebInspectorUI.vcxproj.filters:392
> -    <None Include="..\UserInterface\DetailsSidebarPanel.css">
> +    <None Include="..\UserInterface\SidebarPanel.css">
>        <Filter>UserInterface</Filter>
>      </None>

Oops, these few lines should be removed I guess, cause I sorted it below.
Comment 5 Timothy Hatcher 2014-11-07 15:02:29 PST 
Comment on attachment 241208 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/ApplicationCacheDetailsSidebarPanel.js:30
> -    this.element.classList.add(WebInspector.ApplicationCacheDetailsSidebarPanel.StyleClassName);
> +    this.contentElement.classList.add(WebInspector.ApplicationCacheDetailsSidebarPanel.StyleClassName);

I think these should stay on element. I think it will mess up selectors otherwise. (Not in this case, but CSS does this: .sidebar > .panel.details.css-style)

> Source/WebInspectorUI/UserInterface/Views/DOMNodeDetailsSidebarPanel.js:33
> -    this.element.classList.add(WebInspector.DOMNodeDetailsSidebarPanel.StyleClassName);
> +    this.contentElement.classList.add(WebInspector.DOMNodeDetailsSidebarPanel.StyleClassName);

Ditto.

> Source/WebInspectorUI/UserInterface/Views/LayerTreeDetailsSidebarPanel.cssSource/WebInspectorUI/UserInterface/Views/LayerTreeSidebarPanel.css:26
> +.sidebar > .layer-tree.panel > .content {

CSSStyleDetailsSidebarPanel.css and other do this:

.sidebar > .panel.details.css-style

> Source/WebInspectorUI/UserInterface/Views/LayerTreeDetailsSidebarPanel.jsSource/WebInspectorUI/UserInterface/Views/LayerTreeSidebarPanel.js:26
> -WebInspector.LayerTreeSidebarPanel = function() {
> +WebInspector.LayerTreeDetailsSidebarPanel = function()

YES!

> Source/WebInspectorUI/UserInterface/Views/LayerTreeDetailsSidebarPanel.jsSource/WebInspectorUI/UserInterface/Views/LayerTreeSidebarPanel.js:32
> -    this.element.classList.add(WebInspector.LayerTreeSidebarPanel.StyleClassName);
> +    this.element.classList.add(WebInspector.LayerTreeDetailsSidebarPanel.StyleClassName);

You kept it on element here.
Comment 6 Joseph Pecoraro 2014-11-07 15:33:42 PST 
I don't know how to fix the GTK build error:
ninja: error: '../../Source/WebInspectorUI/UserInterface/Views/LayerTreeSidebarPanel.css', needed by 'DerivedSources/webkit2gtk/InspectorGResourceBundle.xml', missing and no known rule to make it

I think GTK needs to do a clean build. LayerTreeSidebarPanel.css was renamed in this patch. I see no explicit references to it in the Source tree.
Comment 7 Joseph Pecoraro 2014-11-07 15:34:05 PST 
(In reply to comment #3)
> Created attachment 241208 [details]
> [PATCH] Proposed Fix
> 
> I think I saw one regression from this.
> 
> * STEPS TO REPRODUCE
> 1. Inspect apple.com
> 2. Show Styles for <body>
> 3. Show Computed section, with "Show All" enabled
> 4. Scroll way down in the sidebar
> 5. Uncheck "Show All"
>   => doesn't scroll the new content into view appropriately
> 
> But right now I think this is a WebCore issue.

Yes, this is a WebCore issue: bug 138525
Comment 8 Joseph Pecoraro 2014-11-07 15:37:53 PST 
http://trac.webkit.org/changeset/175767