[webkit-reviews] review granted: [Bug 191441] [iOS] Implement safe browsing in WebKit : [Attachment 354452] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Nov 12 10:41:56 PST 2018


Tim Horton <thorton at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 191441: [iOS] Implement safe browsing in WebKit
https://bugs.webkit.org/show_bug.cgi?id=191441

Attachment 354452: Patch

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




--- Comment #14 from Tim Horton <thorton at apple.com> ---
Comment on attachment 354452
  --> https://bugs.webkit.org/attachment.cgi?id=354452
Patch

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

> Source/WebKit/UIProcess/API/C/mac/WKContextPrivateMac.mm:176
> -    return DEFAULT_SAFE_BROWSING_ENABLED;
> +    return true;

Still don't understand why this doesn't follow the preference (or why you have
a preference if you're not going to follow it), but OK

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:3604
> +    if (_safeBrowsingWarning)

Why is this not in the Impl? This way it's going to make the eventual plop
harder, and also means WKView doesn't get it.

And these aren't used on iOS so they can't be relevant to this patch.

> Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:169
> +    [exclamationPoint appendBezierPathWithArcWithCenter: { centerX,
lineBottomCenterY } radius:lineRadius startAngle:0 endAngle:180 clockwise:YES];

Choose one space-after-colon style and stick to it (and it should be "no").


More information about the webkit-reviews mailing list