[webkit-reviews] review granted: [Bug 177896] Web Inspector: Network Tab - Headers Detail View : [Attachment 323030] [PATCH] Proposed Fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 6 15:10:29 PDT 2017


Devin Rousso <webkit at devinrousso.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 177896: Web Inspector: Network Tab - Headers Detail View
https://bugs.webkit.org/show_bug.cgi?id=177896

Attachment 323030: [PATCH] Proposed Fix

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




--- Comment #29 from Devin Rousso <webkit at devinrousso.com> ---
Comment on attachment 323030
  --> https://bugs.webkit.org/attachment.cgi?id=323030
[PATCH] Proposed Fix

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

r=me

> Source/WebInspectorUI/ChangeLog:32
> +	   New event whenever metrics change. This is the first event that will
allow
> +	   a client to react to a Protocol change.

What does this mean?  I think this can be reworded a bit for clarity.

> Source/WebInspectorUI/UserInterface/Views/NetworkResourceDetailView.css:82
> +    padding: 0 20px 20px;

I tried messing with this a bit, and I think you could decrease the horizontal
padding here to 10px and the vertical padding of `.resource-details > section`
(ResourceDetailsSection.css), and you'll get a bit more space to work with. 
Just a thought :)

> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.css:26
> +body[dir] .resource-headers > section > .details {

I'm confused as to what the purpose of `body[dir]` is?	Is it necessary?

> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.css:53
> +    /* Don't include indents in RTL */

I tried this locally, and it does seem really funky.  I wonder why it does this
:/

> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:37
> +	   this._resource.addEventListener(WI.Resource.Event.MetricsDidChange,
this._resourceMetricsDidChange, this);
> +	  
this._resource.addEventListener(WI.Resource.Event.RequestHeadersDidChange,
this._resourceRequestHeadersDidChange, this);
> +	   this._resource.addEventListener(WI.Resource.Event.ResponseReceived,
this._resourceResponseReceived, this);

Can these event listeners be moved to initialLayout()?	It seems they all call
`needsLayout()` anyway.


More information about the webkit-reviews mailing list