[webkit-reviews] review granted: [Bug 191539] Web Inspector: Network: show secure connection details per-request : [Attachment 357511] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 17 18:25:53 PST 2018

Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 191539: Web Inspector: Network: show secure connection details per-request

Attachment 357511: Patch


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

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


> Source/JavaScriptCore/inspector/protocol/Network.json:97
> +		   { "name": "securityConnection", "$ref":
"Security.Connection", "optional": true, "description": "Connection information
for the completed request." }

Why not just name this `connection`?

> Source/WebCore/PAL/pal/spi/cf/CFNetworkSPI.h:214
> +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) ||

Alexey was just talking about moving these to USE/HAVE macros. For now I think
its okay as is, I actually prefer to see this info at these sites.

> Source/WebInspectorUI/UserInterface/Views/ResourceSecurityContentView.js:197
> +	   this._connectionSection.appendKeyValuePair(WI.UIString("Protocol"),
> +	   this._connectionSection.appendKeyValuePair(WI.UIString("Cipher"),

The backend / protocol allows for only one of these properties. There should
probably be a default value such as an emdash.

    connection.protocol || emDash

More information about the webkit-reviews mailing list