[iOS] Implement safe browsing in WebKit
Created attachment 354284 [details] Patch
Comment on attachment 354284 [details] Patch The UI isn't quite right on this first patch, but it works!
Created attachment 354348 [details] Patch
Comment on attachment 354348 [details] Patch iOS UI still isn't quite right, but much closer!
Created attachment 354402 [details] Patch
So close! I need to use UIUserInterfaceSizeClass instead of hard-coding a narrow width, listen for orientation transitions, and make it scroll on iOS, but other than that it's good! Preliminary reviews welcome.
Created attachment 354436 [details] Patch
Comment on attachment 354436 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354436&action=review > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:279 > + NSUnderlineStyleAttributeName: @(NSUnderlineStyleSingle), > + NSUnderlineColorAttributeName: [UIColor whiteColor], > + NSForegroundColorAttributeName: colorForItem(item, warning), Nit - extra spaces after the : > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:316 > +@interface WKSafeBrowsingTextViewDelegate : NSObject<NSTextViewDelegate> It's not common practice in ObjC to create a new instance of a class whose sole purpose is to be a delegate. I would've thought that the safe browsing view should be the text view delegate. Then, you wouldn't need this extra bit of plumbing when handling a click: [((WKSafeBrowsingTextView *)textView)->_warning clickedOnLink:link]; > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:456 > + for (ViewType *subview in self.subviews) { It seems strange to go digging around in the view hierarchy in search of a view that we created and added ourselves. Can we instead store a pointer to the WKSafeBrowsingTextView above and just invalidate its intrinsic content size here? > Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:560 > + constexpr auto noHeightConstraint = 1000000; Perhaps CGFLOAT_MAX instead of an arbitrary large number?
Created attachment 354442 [details] Patch
Created attachment 354446 [details] Patch
Created attachment 354449 [details] Patch
Created attachment 354450 [details] Patch
Created attachment 354452 [details] Patch
Comment on attachment 354452 [details] 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").
Created attachment 354590 [details] Patch
http://trac.webkit.org/r238115
<rdar://problem/46010524>
http://trac.webkit.org/r238133