<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[192037] trunk/Source/WebKit2</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/192037">192037</a></dd>
<dt>Author</dt> <dd>bdakin@apple.com</dd>
<dt>Date</dt> <dd>2015-11-04 14:34:01 -0800 (Wed, 04 Nov 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2SharedInteractionInformationAtPositioncpp">trunk/Source/WebKit2/Shared/InteractionInformationAtPosition.cpp</a></li>
<li><a href="#trunkSourceWebKit2SharedInteractionInformationAtPositionh">trunk/Source/WebKit2/Shared/InteractionInformationAtPosition.h</a></li>
<li><a href="#trunkSourceWebKit2UIProcessiosWKContentViewInteractionmm">trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebPageiosWebPageIOSmm">trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (192036 => 192037)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+2015-11-04 Beth Dakin <bdakin@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):
+
</ins><span class="cx"> 2015-11-04 Wenson Hsieh <wenson_hsieh@apple.com>
</span><span class="cx">
</span><span class="cx"> Fix crashing fast-clicking WK2 tests on iOS
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedInteractionInformationAtPositioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/InteractionInformationAtPosition.cpp (192036 => 192037)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -39,8 +39,10 @@
</span><span class="cx"> encoder << isSelectable;
</span><span class="cx"> encoder << isNearMarkedText;
</span><span class="cx"> encoder << touchCalloutEnabled;
</span><ins>+ encoder << isLink;
+ encoder << isImage;
</ins><span class="cx"> encoder << isAnimatedImage;
</span><del>- encoder << clickableElementName;
</del><ins>+ encoder << isClickableElement;
</ins><span class="cx"> encoder << url;
</span><span class="cx"> encoder << imageURL;
</span><span class="cx"> encoder << title;
</span><span class="lines">@@ -70,10 +72,16 @@
</span><span class="cx"> if (!decoder.decode(result.touchCalloutEnabled))
</span><span class="cx"> return false;
</span><span class="cx">
</span><ins>+ if (!decoder.decode(result.isLink))
+ return false;
+
+ if (!decoder.decode(result.isImage))
+ return false;
+
</ins><span class="cx"> if (!decoder.decode(result.isAnimatedImage))
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- if (!decoder.decode(result.clickableElementName))
</del><ins>+ if (!decoder.decode(result.isClickableElement))
</ins><span class="cx"> return false;
</span><span class="cx">
</span><span class="cx"> if (!decoder.decode(result.url))
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedInteractionInformationAtPositionh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/InteractionInformationAtPosition.h (192036 => 192037)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -43,8 +43,10 @@
</span><span class="cx"> bool isSelectable { false };
</span><span class="cx"> bool isNearMarkedText { false };
</span><span class="cx"> bool touchCalloutEnabled { true };
</span><ins>+ bool isLink { false };
+ bool isImage { false };
</ins><span class="cx"> bool isAnimatedImage { false };
</span><del>- String clickableElementName;
</del><ins>+ bool isClickableElement { false };
</ins><span class="cx"> String url;
</span><span class="cx"> String imageURL;
</span><span class="cx"> String title;
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessiosWKContentViewInteractionmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm (192036 => 192037)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1049,10 +1049,10 @@
</span><span class="cx"> if (!_positionInformation.touchCalloutEnabled)
</span><span class="cx"> return nil;
</span><span class="cx">
</span><del>- if (equalIgnoringCase(_positionInformation.clickableElementName, "IMG"))
</del><ins>+ if (_positionInformation.isImage)
</ins><span class="cx"> return @selector(_showImageSheet);
</span><span class="cx">
</span><del>- if (equalIgnoringCase(_positionInformation.clickableElementName, "A")) {
</del><ins>+ if (_positionInformation.isLink) {
</ins><span class="cx"> NSURL *targetURL = [NSURL URLWithString:_positionInformation.url];
</span><span class="cx"> if ([[getDDDetectionControllerClass() tapAndHoldSchemes] containsObject:[targetURL scheme]])
</span><span class="cx"> return @selector(_showDataDetectorsSheet);
</span><span class="lines">@@ -1094,7 +1094,7 @@
</span><span class="cx"> // This is a different node than the assisted one.
</span><span class="cx"> // Prevent the gesture if there is no node.
</span><span class="cx"> // Allow the gesture if it is a node that wants highlight or if there is an action for it.
</span><del>- if (_positionInformation.clickableElementName.isNull())
</del><ins>+ if (!_positionInformation.isClickableElement)
</ins><span class="cx"> return NO;
</span><span class="cx"> return [self _actionForLongPress] != nil;
</span><span class="cx"> } else {
</span><span class="lines">@@ -1208,7 +1208,7 @@
</span><span class="cx"> _isTapHighlightIDValid = YES;
</span><span class="cx"> break;
</span><span class="cx"> case UIGestureRecognizerStateEnded:
</span><del>- if (_highlightLongPressCanClick && !_positionInformation.clickableElementName.isEmpty()) {
</del><ins>+ if (_highlightLongPressCanClick && _positionInformation.isClickableElement) {
</ins><span class="cx"> [self _attemptClickAtLocation:[gestureRecognizer startPoint]];
</span><span class="cx"> [self _finishInteraction];
</span><span class="cx"> } else
</span><span class="lines">@@ -3434,11 +3434,11 @@
</span><span class="cx"> return NO;
</span><span class="cx">
</span><span class="cx"> [self ensurePositionInformationIsUpToDate:position];
</span><del>- if (equalIgnoringCase(_positionInformation.clickableElementName, "A") && equalIgnoringCase(_positionInformation.clickableElementName, "IMG"))
</del><ins>+ if (!_positionInformation.isLink && !_positionInformation.isImage)
</ins><span class="cx"> return NO;
</span><span class="cx">
</span><span class="cx"> String absoluteLinkURL = _positionInformation.url;
</span><del>- if (equalIgnoringCase(_positionInformation.clickableElementName, "A")) {
</del><ins>+ if (_positionInformation.isLink) {
</ins><span class="cx"> if (absoluteLinkURL.isEmpty())
</span><span class="cx"> return NO;
</span><span class="cx"> if (WebCore::protocolIsInHTTPFamily(absoluteLinkURL))
</span><span class="lines">@@ -3457,8 +3457,8 @@
</span><span class="cx">
</span><span class="cx"> id <WKUIDelegatePrivate> uiDelegate = static_cast<id <WKUIDelegatePrivate>>([_webView UIDelegate]);
</span><span class="cx"> BOOL supportsImagePreview = [uiDelegate respondsToSelector:@selector(_webView:commitPreviewedImageWithURL:)];
</span><del>- BOOL canShowImagePreview = equalIgnoringCase(_positionInformation.clickableElementName, "IMG") && supportsImagePreview;
- BOOL canShowLinkPreview = equalIgnoringCase(_positionInformation.clickableElementName, "A") || canShowImagePreview;
</del><ins>+ BOOL canShowImagePreview = _positionInformation.isImage && supportsImagePreview;
+ BOOL canShowLinkPreview = _positionInformation.isLink || canShowImagePreview;
</ins><span class="cx"> BOOL useImageURLForLink = NO;
</span><span class="cx">
</span><span class="cx"> if (canShowImagePreview && _positionInformation.isAnimatedImage) {
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPageiosWebPageIOSmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm (192036 => 192037)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -2175,10 +2175,9 @@
</span><span class="cx"> }
</span><span class="cx"> bool elementIsLinkOrImage = false;
</span><span class="cx"> if (hitNode) {
</span><del>- info.clickableElementName = hitNode->nodeName();
-
</del><span class="cx"> Element* element = is<Element>(*hitNode) ? downcast<Element>(hitNode) : nullptr;
</span><span class="cx"> if (element) {
</span><ins>+ info.isClickableElement = true;
</ins><span class="cx"> Element* linkElement = nullptr;
</span><span class="cx"> if (element->renderer() && element->renderer()->isRenderImage()) {
</span><span class="cx"> elementIsLinkOrImage = true;
</span><span class="lines">@@ -2190,6 +2189,8 @@
</span><span class="cx">
</span><span class="cx"> if (elementIsLinkOrImage) {
</span><span class="cx"> if (linkElement) {
</span><ins>+ info.isLink = true;
+
</ins><span class="cx"> // Ensure that the image contains at most 600K pixels, so that it is not too big.
</span><span class="cx"> if (RefPtr<WebImage> snapshot = snapshotNode(*element, SnapshotOptionsShareable, 600 * 1024))
</span><span class="cx"> info.image = snapshot->bitmap();
</span><span class="lines">@@ -2205,6 +2206,7 @@
</span><span class="cx"> info.linkIndicator = textIndicator->data();
</span><span class="cx"> }
</span><span class="cx"> } else if (element->renderer() && element->renderer()->isRenderImage()) {
</span><ins>+ info.isImage = true;
</ins><span class="cx"> auto& renderImage = downcast<RenderImage>(*(element->renderer()));
</span><span class="cx"> if (renderImage.cachedImage() && !renderImage.cachedImage()->errorOccurred()) {
</span><span class="cx"> if (Image* image = renderImage.cachedImage()->imageForRenderer(&renderImage)) {
</span></span></pre>
</div>
</div>
</body>
</html>