Title: [192037] trunk/Source/WebKit2
Revision
192037
Author
bda...@apple.com
Date
2015-11-04 14:34:01 -0800 (Wed, 04 Nov 2015)

Log Message

Link preview doesn't work on XHTML pages with Content-Type header as 
`application/xhtml+xml`
https://bugs.webkit.org/show_bug.cgi?id=150740
-and corresponding-
rdar://problem/23063585

Reviewed by Darin Adler.

My original fix for this bug was incorrect in the presence of non-HTML 
elements that happen to have the same local name as HTML elements. Since it 
seems silly to have all of this logic in the UI process to determine whether 
to treat something as a link or an image, this patch fixes the bug by adding 
isLink and isImage to InteractionInformationAtPosition in order to simplify 
everything. The only remaining uses of clickableElementName just use it to 
compare against isNull and isEmpty, so that can be a bool too.

Add isLink and isImage, and turn clickableElementName into isClickableElement
* Shared/InteractionInformationAtPosition.cpp:
(WebKit::InteractionInformationAtPosition::encode):
(WebKit::InteractionInformationAtPosition::decode):
* Shared/InteractionInformationAtPosition.h:

Use the new isLink, isImage, and isClickableElement
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _actionForLongPress]):
(-[WKContentView gestureRecognizerShouldBegin:]):
(-[WKContentView _highlightLongPressRecognized:]):
(-[WKContentView _interactionShouldBeginFromPreviewItemController:forPosition:]):
(-[WKContentView _dataForPreviewItemController:atPosition:type:]):

Set everything correctly.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::getPositionInformation):

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (192036 => 192037)


--- trunk/Source/WebKit2/ChangeLog	2015-11-04 22:21:25 UTC (rev 192036)
+++ trunk/Source/WebKit2/ChangeLog	2015-11-04 22:34:01 UTC (rev 192037)
@@ -1,3 +1,39 @@
+2015-11-04  Beth Dakin  <bda...@apple.com>
+
+        Link preview doesn't work on XHTML pages with Content-Type header as 
+        `application/xhtml+xml`
+        https://bugs.webkit.org/show_bug.cgi?id=150740
+        -and corresponding-
+        rdar://problem/23063585
+
+        Reviewed by Darin Adler.
+
+        My original fix for this bug was incorrect in the presence of non-HTML 
+        elements that happen to have the same local name as HTML elements. Since it 
+        seems silly to have all of this logic in the UI process to determine whether 
+        to treat something as a link or an image, this patch fixes the bug by adding 
+        isLink and isImage to InteractionInformationAtPosition in order to simplify 
+        everything. The only remaining uses of clickableElementName just use it to 
+        compare against isNull and isEmpty, so that can be a bool too.
+
+        Add isLink and isImage, and turn clickableElementName into isClickableElement
+        * Shared/InteractionInformationAtPosition.cpp:
+        (WebKit::InteractionInformationAtPosition::encode):
+        (WebKit::InteractionInformationAtPosition::decode):
+        * Shared/InteractionInformationAtPosition.h:
+
+        Use the new isLink, isImage, and isClickableElement
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _actionForLongPress]):
+        (-[WKContentView gestureRecognizerShouldBegin:]):
+        (-[WKContentView _highlightLongPressRecognized:]):
+        (-[WKContentView _interactionShouldBeginFromPreviewItemController:forPosition:]):
+        (-[WKContentView _dataForPreviewItemController:atPosition:type:]):
+
+        Set everything correctly.
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::getPositionInformation):
+
 2015-11-04  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Fix crashing fast-clicking WK2 tests on iOS

Modified: trunk/Source/WebKit2/Shared/InteractionInformationAtPosition.cpp (192036 => 192037)


--- trunk/Source/WebKit2/Shared/InteractionInformationAtPosition.cpp	2015-11-04 22:21:25 UTC (rev 192036)
+++ trunk/Source/WebKit2/Shared/InteractionInformationAtPosition.cpp	2015-11-04 22:34:01 UTC (rev 192037)
@@ -39,8 +39,10 @@
     encoder << isSelectable;
     encoder << isNearMarkedText;
     encoder << touchCalloutEnabled;
+    encoder << isLink;
+    encoder << isImage;
     encoder << isAnimatedImage;
-    encoder << clickableElementName;
+    encoder << isClickableElement;
     encoder << url;
     encoder << imageURL;
     encoder << title;
@@ -70,10 +72,16 @@
     if (!decoder.decode(result.touchCalloutEnabled))
         return false;
 
+    if (!decoder.decode(result.isLink))
+        return false;
+
+    if (!decoder.decode(result.isImage))
+        return false;
+
     if (!decoder.decode(result.isAnimatedImage))
         return false;
     
-    if (!decoder.decode(result.clickableElementName))
+    if (!decoder.decode(result.isClickableElement))
         return false;
 
     if (!decoder.decode(result.url))

Modified: trunk/Source/WebKit2/Shared/InteractionInformationAtPosition.h (192036 => 192037)


--- trunk/Source/WebKit2/Shared/InteractionInformationAtPosition.h	2015-11-04 22:21:25 UTC (rev 192036)
+++ trunk/Source/WebKit2/Shared/InteractionInformationAtPosition.h	2015-11-04 22:34:01 UTC (rev 192037)
@@ -43,8 +43,10 @@
     bool isSelectable { false };
     bool isNearMarkedText { false };
     bool touchCalloutEnabled { true };
+    bool isLink { false };
+    bool isImage { false };
     bool isAnimatedImage { false };
-    String clickableElementName;
+    bool isClickableElement { false };
     String url;
     String imageURL;
     String title;

Modified: trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm (192036 => 192037)


--- trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm	2015-11-04 22:21:25 UTC (rev 192036)
+++ trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm	2015-11-04 22:34:01 UTC (rev 192037)
@@ -1049,10 +1049,10 @@
     if (!_positionInformation.touchCalloutEnabled)
         return nil;
 
-    if (equalIgnoringCase(_positionInformation.clickableElementName, "IMG"))
+    if (_positionInformation.isImage)
         return @selector(_showImageSheet);
 
-    if (equalIgnoringCase(_positionInformation.clickableElementName, "A")) {
+    if (_positionInformation.isLink) {
         NSURL *targetURL = [NSURL URLWithString:_positionInformation.url];
         if ([[getDDDetectionControllerClass() tapAndHoldSchemes] containsObject:[targetURL scheme]])
             return @selector(_showDataDetectorsSheet);
@@ -1094,7 +1094,7 @@
             // This is a different node than the assisted one.
             // Prevent the gesture if there is no node.
             // Allow the gesture if it is a node that wants highlight or if there is an action for it.
-            if (_positionInformation.clickableElementName.isNull())
+            if (!_positionInformation.isClickableElement)
                 return NO;
             return [self _actionForLongPress] != nil;
         } else {
@@ -1208,7 +1208,7 @@
         _isTapHighlightIDValid = YES;
         break;
     case UIGestureRecognizerStateEnded:
-        if (_highlightLongPressCanClick && !_positionInformation.clickableElementName.isEmpty()) {
+        if (_highlightLongPressCanClick && _positionInformation.isClickableElement) {
             [self _attemptClickAtLocation:[gestureRecognizer startPoint]];
             [self _finishInteraction];
         } else
@@ -3434,11 +3434,11 @@
         return NO;
 
     [self ensurePositionInformationIsUpToDate:position];
-    if (equalIgnoringCase(_positionInformation.clickableElementName, "A") && equalIgnoringCase(_positionInformation.clickableElementName, "IMG"))
+    if (!_positionInformation.isLink && !_positionInformation.isImage)
         return NO;
     
     String absoluteLinkURL = _positionInformation.url;
-    if (equalIgnoringCase(_positionInformation.clickableElementName, "A")) {
+    if (_positionInformation.isLink) {
         if (absoluteLinkURL.isEmpty())
             return NO;
         if (WebCore::protocolIsInHTTPFamily(absoluteLinkURL))
@@ -3457,8 +3457,8 @@
 
     id <WKUIDelegatePrivate> uiDelegate = static_cast<id <WKUIDelegatePrivate>>([_webView UIDelegate]);
     BOOL supportsImagePreview = [uiDelegate respondsToSelector:@selector(_webView:commitPreviewedImageWithURL:)];
-    BOOL canShowImagePreview = equalIgnoringCase(_positionInformation.clickableElementName, "IMG") && supportsImagePreview;
-    BOOL canShowLinkPreview = equalIgnoringCase(_positionInformation.clickableElementName, "A") || canShowImagePreview;
+    BOOL canShowImagePreview = _positionInformation.isImage && supportsImagePreview;
+    BOOL canShowLinkPreview = _positionInformation.isLink || canShowImagePreview;
     BOOL useImageURLForLink = NO;
 
     if (canShowImagePreview && _positionInformation.isAnimatedImage) {

Modified: trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm (192036 => 192037)


--- trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm	2015-11-04 22:21:25 UTC (rev 192036)
+++ trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm	2015-11-04 22:34:01 UTC (rev 192037)
@@ -2175,10 +2175,9 @@
     }
     bool elementIsLinkOrImage = false;
     if (hitNode) {
-        info.clickableElementName = hitNode->nodeName();
-
         Element* element = is<Element>(*hitNode) ? downcast<Element>(hitNode) : nullptr;
         if (element) {
+            info.isClickableElement = true;
             Element* linkElement = nullptr;
             if (element->renderer() && element->renderer()->isRenderImage()) {
                 elementIsLinkOrImage = true;
@@ -2190,6 +2189,8 @@
 
             if (elementIsLinkOrImage) {
                 if (linkElement) {
+                    info.isLink = true;
+
                     // Ensure that the image contains at most 600K pixels, so that it is not too big.
                     if (RefPtr<WebImage> snapshot = snapshotNode(*element, SnapshotOptionsShareable, 600 * 1024))
                         info.image = snapshot->bitmap();
@@ -2205,6 +2206,7 @@
                             info.linkIndicator = textIndicator->data();
                     }
                 } else if (element->renderer() && element->renderer()->isRenderImage()) {
+                    info.isImage = true;
                     auto& renderImage = downcast<RenderImage>(*(element->renderer()));
                     if (renderImage.cachedImage() && !renderImage.cachedImage()->errorOccurred()) {
                         if (Image* image = renderImage.cachedImage()->imageForRenderer(&renderImage)) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to