- Revision
- 247342
- Author
- timothy_hor...@apple.com
- Date
- 2019-07-10 21:09:52 -0700 (Wed, 10 Jul 2019)
Log Message
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.
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.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (247341 => 247342)
--- trunk/Source/WebKit/ChangeLog 2019-07-11 02:09:52 UTC (rev 247341)
+++ trunk/Source/WebKit/ChangeLog 2019-07-11 04:09:52 UTC (rev 247342)
@@ -1,3 +1,26 @@
+2019-07-10 Tim Horton <timothy_hor...@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 <d...@apple.com>
Safari’s context menu actions are missing options
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (247341 => 247342)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2019-07-11 02:09:52 UTC (rev 247341)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2019-07-11 04:09:52 UTC (rev 247342)
@@ -2523,7 +2523,7 @@
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 @@
info.isLink = true;
info.url = ""
- linkIndicatorPositionInformation(page, element, *linkElement, request, info);
+ linkIndicatorPositionInformation(page, *linkElement, request, info);
#if ENABLE(DATA_DETECTION)
dataDetectorLinkPositionInformation(element, info);
#endif
@@ -2662,8 +2662,9 @@
// 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 = ""
} else {
@@ -2725,15 +2726,9 @@
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)) {
Modified: trunk/Tools/ChangeLog (247341 => 247342)
--- trunk/Tools/ChangeLog 2019-07-11 02:09:52 UTC (rev 247341)
+++ trunk/Tools/ChangeLog 2019-07-11 04:09:52 UTC (rev 247342)
@@ -1,3 +1,15 @@
+2019-07-10 Tim Horton <timothy_hor...@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 <d...@apple.com>
Support MacCatalyst in run-webkit-app
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKRequestActivatedElementInfo.mm (247341 => 247342)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKRequestActivatedElementInfo.mm 2019-07-11 02:09:52 UTC (rev 247341)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKRequestActivatedElementInfo.mm 2019-07-11 04:09:52 UTC (rev 247342)
@@ -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 @@
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)]);
