[webkit-reviews] review granted: [Bug 190440] WebKit Inspector: Expose Server Timing Response Headers in Network Tab : [Attachment 352214] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted cvazac at gmail.com's request for
review:
Bug 190440: WebKit Inspector: Expose Server Timing Response Headers in Network
Tab
https://bugs.webkit.org/show_bug.cgi?id=190440

Attachment 352214: Patch

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




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

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

r=me!

> Source/WebInspectorUI/ChangeLog:9
> +	   * UserInterface/Main.html:

You should regenerate the ChangeLog, this now has edits to new files like
Test.html.

> Source/WebInspectorUI/UserInterface/Views/ResourceTimingBreakdownView.js:184
> +	       this._appendHeaderRow(WI.UIString("Server Timing"));

This lost the colon. Should we add it back? I only suggested it because it
matches the other header rows.

> LayoutTests/inspector/unit-tests/server-timing-entry.html:63
> +	       // spaces
> +	       testServerTimingHeader(`metric ; `, [{"name":"metric"}]);

Maybe have a test for leading whitespace. Seems like it would be stripped via
`consumeToken()` and work just fine.