[webkit-reviews] review granted: [Bug 172470] Web Inspector: use initialLayout for NetworkSidebarPanel : [Attachment 310910] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 22 21:40:42 PDT 2017


Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 172470: Web Inspector: use initialLayout for NetworkSidebarPanel
https://bugs.webkit.org/show_bug.cgi?id=172470

Attachment 310910: Patch

https://bugs.webkit.org/attachment.cgi?id=310910&action=review




--- Comment #4 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 310910
  --> https://bugs.webkit.org/attachment.cgi?id=310910
Patch

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

r=me, but with concerns

> Source/WebInspectorUI/UserInterface/Views/NetworkSidebarPanel.js:41
>	  
this.contentBrowser.addEventListener(WebInspector.ContentBrowser.Event.CurrentC
ontentViewDidChange, this._contentBrowserCurrentContentViewDidChange, this);

As was mentioned previously, it doesn't make sense to me why this is not in
initialLayout. It seems potentially problematic keeping it separate. It is
quite small right now but I do worry about future changes adding code in there
that is unsafe unless performed after initial layout.

> Source/WebInspectorUI/UserInterface/Views/NetworkSidebarPanel.js:82
> +	   var scopeItemPrefix = "network-sidebar-";

Style: Lets upgrade these to `let` now.


More information about the webkit-reviews mailing list