Safe browsing warning text needs to be visible on High Sierra
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Nov 2018 00:15:01 +0000 (00:15 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Nov 2018 00:15:01 +0000 (00:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192022

Reviewed by Tim Horton.

Source/WebKit:

Something about AppKit, autolayout, view insertion order, and NSTextView makes the text lay
out with initial size of {0, 0} on High Sierra. Using an NSTextField instead makes the details visible.

Covered by an API test.

* UIProcess/Cocoa/WKSafeBrowsingWarning.h:
* UIProcess/Cocoa/WKSafeBrowsingWarning.mm:
(makeLabel):
(-[WKSafeBrowsingWarning addContent]):
(-[WKSafeBrowsingWarning showDetailsClicked]):
(-[WKSafeBrowsingWarning layoutText]):
(makeTitleLabel): Deleted.

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm:
(TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238585 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.h
Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm

index 5e10d09..daef2e9 100644 (file)
@@ -1,3 +1,23 @@
+2018-11-27  Alex Christensen  <achristensen@webkit.org>
+
+        Safe browsing warning text needs to be visible on High Sierra
+        https://bugs.webkit.org/show_bug.cgi?id=192022
+
+        Reviewed by Tim Horton.
+
+        Something about AppKit, autolayout, view insertion order, and NSTextView makes the text lay
+        out with initial size of {0, 0} on High Sierra. Using an NSTextField instead makes the details visible.
+
+        Covered by an API test.
+
+        * UIProcess/Cocoa/WKSafeBrowsingWarning.h:
+        * UIProcess/Cocoa/WKSafeBrowsingWarning.mm:
+        (makeLabel):
+        (-[WKSafeBrowsingWarning addContent]):
+        (-[WKSafeBrowsingWarning showDetailsClicked]):
+        (-[WKSafeBrowsingWarning layoutText]):
+        (makeTitleLabel): Deleted.
+
 2018-11-27  Tim Horton  <timothy_horton@apple.com>
 
         WKNavigation.AutomaticViewReloadAfterWebProcessCrash asserts after r238538
index 16025d9..07e149a 100644 (file)
@@ -57,7 +57,7 @@ using RectType = CGRect;
 @package
     CompletionHandler<void(Variant<WebKit::ContinueUnsafeLoad, WebCore::URL>&&)> _completionHandler;
     RefPtr<const WebKit::SafeBrowsingWarning> _warning;
-    RetainPtr<NSMutableArray<WKSafeBrowsingTextView *>> _textViews;
+    WeakObjCPtr<WKSafeBrowsingTextView> _details;
 }
 
 - (instancetype)initWithFrame:(RectType)frame safeBrowsingWarning:(const WebKit::SafeBrowsingWarning&)warning completionHandler:(CompletionHandler<void(Variant<WebKit::ContinueUnsafeLoad, WebCore::URL>&&)>&&)completionHandler;
index bed1cfe..afd47d0 100644 (file)
@@ -197,12 +197,8 @@ static ButtonType *makeButton(WarningItem item, WKSafeBrowsingWarning *warning,
 #endif
 }
 
-static ViewType *makeTitleLabel(NSString *title, ViewType *warning)
+static ViewType *makeLabel(NSAttributedString *attributedString)
 {
-    auto attributedString = [[[NSAttributedString alloc] initWithString:title attributes:@{
-        NSFontAttributeName:[FontType boldSystemFontOfSize:titleSize],
-        NSForegroundColorAttributeName:colorForItem(WarningItem::TitleText, warning)
-    }] autorelease];
 #if PLATFORM(MAC)
     return [NSTextField labelWithAttributedString:attributedString];
 #else
@@ -251,15 +247,19 @@ static void setBackground(ViewType *view, ColorType *color)
 - (void)addContent
 {
     auto exclamationPoint = [[WKSafeBrowsingExclamationPoint new] autorelease];
-    auto title = makeTitleLabel(_warning->title(), self);
-    auto warning = [[[WKSafeBrowsingTextView alloc] initWithAttributedString:[[[NSAttributedString alloc] initWithString:_warning->warning() attributes:@{ NSFontAttributeName:[FontType systemFontOfSize:textSize] }] autorelease] forWarning:self] autorelease];
+    auto title = makeLabel([[[NSAttributedString alloc] initWithString:_warning->title() attributes:@{
+        NSFontAttributeName:[FontType boldSystemFontOfSize:titleSize],
+        NSForegroundColorAttributeName:colorForItem(WarningItem::TitleText, self)
+    }] autorelease]);
+    auto warning = makeLabel([[[NSAttributedString alloc] initWithString:_warning->warning() attributes:@{
+        NSFontAttributeName:[FontType systemFontOfSize:textSize],
+        NSForegroundColorAttributeName:colorForItem(WarningItem::MessageText, self)
+    }] autorelease]);
     auto showDetails = makeButton(WarningItem::ShowDetailsButton, self, @selector(showDetailsClicked));
     auto goBack = makeButton(WarningItem::GoBackButton, self, @selector(goBackClicked));
     auto box = [[ViewType new] autorelease];
     setBackground(box, colorForItem(WarningItem::BoxBackground, self));
     box.layer.cornerRadius = boxCornerRadius;
-    _textViews = adoptNS([NSMutableArray new]);
-    [_textViews addObject:warning];
 
     for (ViewType *view in @[exclamationPoint, title, warning, goBack, showDetails]) {
         view.translatesAutoresizingMaskIntoConstraints = NO;
@@ -306,7 +306,7 @@ static void setBackground(ViewType *view, ColorType *color)
     NSMutableAttributedString *text = [[_warning->details() mutableCopy] autorelease];
     [text addAttributes:@{ NSFontAttributeName:[FontType systemFontOfSize:textSize] } range:NSMakeRange(0, text.length)];
     WKSafeBrowsingTextView *details = [[[WKSafeBrowsingTextView alloc] initWithAttributedString:text forWarning:self] autorelease];
-    [_textViews addObject:details];
+    _details = details;
     ViewType *bottom = [[ViewType new] autorelease];
     setBackground(bottom, colorForItem(WarningItem::BoxBackground, self));
     bottom.layer.cornerRadius = boxCornerRadius;
@@ -358,8 +358,7 @@ static void setBackground(ViewType *view, ColorType *color)
 
 - (void)layoutText
 {
-    for (WKSafeBrowsingTextView *view in _textViews.get())
-        [view invalidateIntrinsicContentSize];
+    [_details invalidateIntrinsicContentSize];
 }
 
 #if PLATFORM(MAC)
index 0989362..c720144 100644 (file)
@@ -1,3 +1,13 @@
+2018-11-27  Alex Christensen  <achristensen@webkit.org>
+
+        Safe browsing warning text needs to be visible on High Sierra
+        https://bugs.webkit.org/show_bug.cgi?id=192022
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/SafeBrowsing.mm:
+        (TEST):
+
 2018-11-27  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Unreviewed, fix the watchOS engineering build
index e1487cd..4b27b4a 100644 (file)
@@ -212,6 +212,7 @@ TEST(SafeBrowsing, VisitUnsafeWebsite)
     auto webView = safeBrowsingView();
     auto warning = [webView _safeBrowsingWarning];
     EXPECT_EQ(warning.subviews.count, 1ull);
+    EXPECT_GT(warning.subviews.firstObject.subviews[2].frame.size.height, 0);
     checkTitleAndClick(warning.subviews.firstObject.subviews[4], "Show Details");
     EXPECT_EQ(warning.subviews.count, 2ull);
     EXPECT_FALSE(committedNavigation);