Long pressing on attachments will crash the WebContent process
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Jul 2019 04:09:52 +0000 (04:09 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Jul 2019 04:09:52 +0000 (04:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199696
<rdar://problem/52920241>

Reviewed by Dean Jackson.

Source/WebKit:

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::linkIndicatorPositionInformation):
(WebKit::elementPositionInformation):
(WebKit::selectionPositionInformation):
(WebKit::WebPage::positionInformation):
Instead of one-off creating a node snapshot for <attachment>, just
use TextIndicator. This way, we get an estimated background color,
paint at the right resolution, etc.

Also, hitNode was often null where we were previously calling
shareableBitmapSnapshotForNode, because it depends on the element
having click event handlers. selectionPositionInformation() re-hit-tests
more permissively to find the <attachment>, so moving this code
inside that function ensures that we don't try to snapshot a null node.

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/WKRequestActivatedElementInfo.mm:
(TestWebKitAPI::TEST):
Add a test that previously crashed.

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

Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKRequestActivatedElementInfo.mm

index 1224f5c..ccddba3 100644 (file)
@@ -1,3 +1,26 @@
+2019-07-10  Tim Horton  <timothy_horton@apple.com>
+
+        Long pressing on attachments will crash the WebContent process
+        https://bugs.webkit.org/show_bug.cgi?id=199696
+        <rdar://problem/52920241>
+
+        Reviewed by Dean Jackson.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::linkIndicatorPositionInformation):
+        (WebKit::elementPositionInformation):
+        (WebKit::selectionPositionInformation):
+        (WebKit::WebPage::positionInformation):
+        Instead of one-off creating a node snapshot for <attachment>, just
+        use TextIndicator. This way, we get an estimated background color,
+        paint at the right resolution, etc.
+
+        Also, hitNode was often null where we were previously calling
+        shareableBitmapSnapshotForNode, because it depends on the element
+        having click event handlers. selectionPositionInformation() re-hit-tests
+        more permissively to find the <attachment>, so moving this code
+        inside that function ensures that we don't try to snapshot a null node.
+
 2019-07-10  Dean Jackson  <dino@apple.com>
 
         Safari‚Äôs context menu actions are missing options
index c7bd8d7..539fbd1 100644 (file)
@@ -2523,7 +2523,7 @@ static void focusedElementPositionInformation(WebPage& page, Element& focusedEle
     info.isNearMarkedText = !(deltaX > kHitAreaWidth || deltaYFromTheTop > kHitAreaHeight || deltaYFromTheBottom > kHitAreaHeight);
 }
 
-static void linkIndicatorPositionInformation(WebPage& page, Element& element, Element& linkElement, const InteractionInformationRequest& request, InteractionInformationAtPosition& info)
+static void linkIndicatorPositionInformation(WebPage& page, Element& linkElement, const InteractionInformationRequest& request, InteractionInformationAtPosition& info)
 {
     if (!request.includeLinkIndicator)
         return;
@@ -2635,7 +2635,7 @@ static void elementPositionInformation(WebPage& page, Element& element, const In
         info.isLink = true;
         info.url = linkElement->document().completeURL(stripLeadingAndTrailingHTMLSpaces(linkElement->getAttribute(HTMLNames::hrefAttr)));
 
-        linkIndicatorPositionInformation(page, element, *linkElement, request, info);
+        linkIndicatorPositionInformation(page, *linkElement, request, info);
 #if ENABLE(DATA_DETECTION)
         dataDetectorLinkPositionInformation(element, info);
 #endif
@@ -2662,8 +2662,9 @@ static void selectionPositionInformation(WebPage& page, const InteractionInforma
     // We don't want to select blocks that are larger than 97% of the visible area of the document.
     if (is<HTMLAttachmentElement>(*hitNode)) {
         info.isAttachment = true;
-        const HTMLAttachmentElement& attachment = downcast<HTMLAttachmentElement>(*hitNode);
+        HTMLAttachmentElement& attachment = downcast<HTMLAttachmentElement>(*hitNode);
         info.title = attachment.attachmentTitle();
+        linkIndicatorPositionInformation(page, attachment, request, info);
         if (attachment.file())
             info.url = URL::fileURLWithFileSystemPath(downcast<HTMLAttachmentElement>(*hitNode).file()->path());
     } else {
@@ -2725,15 +2726,9 @@ InteractionInformationAtPosition WebPage::positionInformation(const InteractionI
             info.image = shareableBitmapSnapshotForNode(element);
     }
 
-    if (!(info.isLink || info.isImage)) {
+    if (!(info.isLink || info.isImage))
         selectionPositionInformation(*this, request, info);
 
-        if (info.isAttachment && request.includeSnapshot) {
-            Element& element = downcast<Element>(*hitNode);
-            info.image = shareableBitmapSnapshotForNode(element);
-        }
-    }
-
     // Prevent the callout bar from showing when tapping on the datalist button.
 #if ENABLE(DATALIST_ELEMENT)
     if (is<HTMLInputElement>(hitNode)) {
index 7f92677..622915e 100644 (file)
@@ -1,3 +1,15 @@
+2019-07-10  Tim Horton  <timothy_horton@apple.com>
+
+        Long pressing on attachments will crash the WebContent process
+        https://bugs.webkit.org/show_bug.cgi?id=199696
+        <rdar://problem/52920241>
+
+        Reviewed by Dean Jackson.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKRequestActivatedElementInfo.mm:
+        (TestWebKitAPI::TEST):
+        Add a test that previously crashed.
+
 2019-07-10  Dean Jackson  <dino@apple.com>
 
         Support MacCatalyst in run-webkit-app
index f85dba1..6e793d0 100644 (file)
@@ -30,6 +30,7 @@
 #import "TestNavigationDelegate.h"
 #import "TestWKWebView.h"
 #import <WebKit/WKWebView.h>
+#import <WebKit/WKWebViewConfigurationPrivate.h>
 #import <WebKit/_WKActivatedElementInfo.h>
 #import <wtf/RetainPtr.h>
 
@@ -134,6 +135,25 @@ TEST(WebKit, RequestActivatedElementInfoForBrokenImage)
     TestWebKitAPI::Util::run(&finished);
 }
 
+TEST(WebKit, RequestActivatedElementInfoForAttachment)
+{
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [configuration _setAttachmentElementEnabled:YES];
+
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration.get()]);
+    [webView loadHTMLString:@"<html><head><meta name='viewport' content='initial-scale=1'></head><body style='margin: 0px;'><attachment  /></body></html>" baseURL:nil];
+    [webView _test_waitForDidFinishNavigation];
+
+    __block bool finished = false;
+    [webView _requestActivatedElementAtPosition:CGPointMake(20, 20) completionBlock:^(_WKActivatedElementInfo *elementInfo) {
+        EXPECT_TRUE(elementInfo.type == _WKActivatedElementTypeAttachment);
+
+        finished = true;
+    }];
+
+    TestWebKitAPI::Util::run(&finished);
+}
+
 TEST(WebKit, RequestActivatedElementInfoWithNestedSynchronousUpdates)
 {
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);