[Webkit-unassigned] [Bug 230173] Web Inspector: Network: "Lock" icon next to domain name in table view should show security information

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=230173

--- Comment #6 from Patrick Angle <pangle at apple.com> ---
Comment on attachment 442778
  --> https://bugs.webkit.org/attachment.cgi?id=442778
Patch

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

Thanks for the patch! This is going to be really handy for quickly getting to certificate info. I've left a few comments, but overall this feels like a good approach.

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:719
> +                let security = entry.resource.security;
> +                if (isEmptyObject(security)) {
> +                    cell.append(domain || emDash);
> +                    return;
> +                }
> +
> +                let certificate = security.certificate;
> +                if (isEmptyObject(certificate) || Object.values(certificate).every((value) => !value)) {
> +                    cell.append(domain || emDash);
> +                    return;
> +                }
> +
> +                lockIconElement.addEventListener("click", (event) => {
> +
> +                    if (WI.NetworkManager.supportsShowCertificate()) {
> +                        entry.resource.showCertificate()
> +                        .catch((error) => {
> +                            console.error(error);
> +                        });
> +                    }
> +                });

While this works, it would be great to not duplicate the `cell.append(domain || emDash)` in multiple places. Instead of early returns here, we could instead do something along the lines of...
```
let certificate = entry.resource.security?.certificate
if (WI.NetworkManager.supportsShowCertificate() && certificate && Object.values(certificate).some((value) => value)) {
    lockIconElement.addEventListener("click", (event) => {
        // Do nothing with thrown exceptions since `showCertificate` will log any thrown exception.
        entry.resource.showCertificate().catch((error) => {});
    });
}
```
Other considerations here are that we should make sure that an appropriate accessibility label is added to the clickable element (see usage of WI.UIString for examples of how we construct localizable strings like this with three parameters: the text, a key, and details for localizers), and we also probably want to add a bit of styling for the cursor to change to a pointer (on macOS it's the hand with index finger extended cursor) for the element similar to how it changes when you hover over the resource name to the left to indicate that clicking will perform an action.

> WebKit.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings:-6
> -	<key>BuildSystemType</key>
> -	<string>Original</string>

Probably unintentional; please remove this change from the patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211029/8d1817ab/attachment.htm>