[webkit-reviews] review granted: [Bug 188871] Implement safe browsing in WebKit : [Attachment 353870] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Nov 5 10:25:41 PST 2018
Tim Horton <thorton at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 188871: Implement safe browsing in WebKit
https://bugs.webkit.org/show_bug.cgi?id=188871
Attachment 353870: Patch
https://bugs.webkit.org/attachment.cgi?id=353870&action=review
--- Comment #54 from Tim Horton <thorton at apple.com> ---
Comment on attachment 353870
--> https://bugs.webkit.org/attachment.cgi?id=353870
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=353870&action=review
> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:6657
> +- (NSView *)_safeBrowsingWarningForTesting
This obviously isn't going to fly on iOS
> Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:200
> ++ (instancetype)viewWithAttributedString:(NSMutableAttributedString
*)attributedString linkTarget:(WKSafeBrowsingWarning *)target;
Why mutable! (Ideally there would be a mutableCopy hiding inside here, and the
caller's string wouldn't get mutated)
> Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:295
> + _completionHandler(makeUnexpected(link));
Alex says "I could use a variant instead of Expected"
> Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:302
> + NSStackView *bottom =box.views.lastObject;
Missing a space after the =
> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:1609
> + weakThis->m_ignoresNonWheelEvents = oldIgnoresNonWheelEvents;
?? do you want to allow scrolling (but not other events) ??
More information about the webkit-reviews
mailing list