<!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>[183282] trunk</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/183282">183282</a></dd>
<dt>Author</dt> <dd>bfulgham@apple.com</dd>
<dt>Date</dt> <dd>2015-04-24 14:53:47 -0700 (Fri, 24 Apr 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>REGRESSION: WebKit2.ActionMenusTest API test fails
https://bugs.webkit.org/show_bug.cgi?id=144149
<rdar://problem/20677770>
Reviewed by Tim Horton.
Source/WebKit2:
Tested by TestWebKitAPI
The 'lookupTextAtLocation' method was not converting the NSEvent coordinates it
was receiving from root view to the plugin view. Consequently, full page PDFs
did hit testing correctly, but an <embed> PDF would not.
Since 'existingSelectionContainsPoint' is exposed as API, I also modified it to
expect 'root view' coordinates and perform the requisite conversions, rather
than expecting 'plugin view' coordinates.
* WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::existingSelectionContainsPoint): Expect 'root view' coordinates
for the input.
(WebKit::PDFPlugin::lookupTextAtLocation): Properly convert the passed 'root view'
coordinates to the PDF's view coordinate space.
* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::performActionMenuHitTestAtLocation): Pass hit point using view coordinates,
not content coordinates.
Tools:
* TestWebKitAPI/Tests/WebKit2/action-menu-targets.html: Revise to use the same test PDF
as the 'ActionMenusPDFTest'.
* TestWebKitAPI/Tests/WebKit2ObjC/ActionMenus.mm:
(TestWebKitAPI::TEST): Unskip the PDF portion of ActionMenusTest, and revise its expected output
to match the hit test on the 'action-menu-target.pdf' test file.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2WebProcessPluginsPDFPDFPluginmm">trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebPagemacWebPageMacmm">trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm</a></li>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsWebKit2actionmenutargetshtml">trunk/Tools/TestWebKitAPI/Tests/WebKit2/action-menu-targets.html</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsWebKit2ObjCActionMenusmm">trunk/Tools/TestWebKitAPI/Tests/WebKit2ObjC/ActionMenus.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (183281 => 183282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2015-04-24 21:39:06 UTC (rev 183281)
+++ trunk/Source/WebKit2/ChangeLog 2015-04-24 21:53:47 UTC (rev 183282)
</span><span class="lines">@@ -1,3 +1,30 @@
</span><ins>+2015-04-24 Brent Fulgham <bfulgham@apple.com>
+
+ REGRESSION: WebKit2.ActionMenusTest API test fails
+ https://bugs.webkit.org/show_bug.cgi?id=144149
+ <rdar://problem/20677770>
+
+ Reviewed by Tim Horton.
+
+ Tested by TestWebKitAPI
+
+ The 'lookupTextAtLocation' method was not converting the NSEvent coordinates it
+ was receiving from root view to the plugin view. Consequently, full page PDFs
+ did hit testing correctly, but an <embed> PDF would not.
+
+ Since 'existingSelectionContainsPoint' is exposed as API, I also modified it to
+ expect 'root view' coordinates and perform the requisite conversions, rather
+ than expecting 'plugin view' coordinates.
+
+ * WebProcess/Plugins/PDF/PDFPlugin.mm:
+ (WebKit::PDFPlugin::existingSelectionContainsPoint): Expect 'root view' coordinates
+ for the input.
+ (WebKit::PDFPlugin::lookupTextAtLocation): Properly convert the passed 'root view'
+ coordinates to the PDF's view coordinate space.
+ * WebProcess/WebPage/mac/WebPageMac.mm:
+ (WebKit::WebPage::performActionMenuHitTestAtLocation): Pass hit point using view coordinates,
+ not content coordinates.
+
</ins><span class="cx"> 2015-04-24 Anders Carlsson <andersca@apple.com>
</span><span class="cx">
</span><span class="cx"> Address a review comment from Joe Pecoraro.
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessPluginsPDFPDFPluginmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm (183281 => 183282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm 2015-04-24 21:39:06 UTC (rev 183281)
+++ trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm 2015-04-24 21:53:47 UTC (rev 183282)
</span><span class="lines">@@ -1846,14 +1846,14 @@
</span><span class="cx"> return [selectionForWord string];
</span><span class="cx"> }
</span><span class="cx">
</span><del>-bool PDFPlugin::existingSelectionContainsPoint(const WebCore::FloatPoint& point) const
</del><ins>+bool PDFPlugin::existingSelectionContainsPoint(const WebCore::FloatPoint& locationInViewCoordinates) const
</ins><span class="cx"> {
</span><span class="cx"> PDFSelection *currentSelection = [m_pdfLayerController currentSelection];
</span><span class="cx"> if (!currentSelection)
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- IntPoint pointInView = convertFromPluginToPDFView(roundedIntPoint(point));
- PDFSelection *selectionForWord = [m_pdfLayerController getSelectionForWordAtPoint:pointInView];
</del><ins>+ IntPoint pointInPDFView = convertFromPluginToPDFView(convertFromRootViewToPlugin(roundedIntPoint(locationInViewCoordinates)));
+ PDFSelection *selectionForWord = [m_pdfLayerController getSelectionForWordAtPoint:pointInPDFView];
</ins><span class="cx">
</span><span class="cx"> NSUInteger currentPageIndex = [m_pdfLayerController currentPageIndex];
</span><span class="cx">
</span><span class="lines">@@ -1896,35 +1896,35 @@
</span><span class="cx">
</span><span class="cx"> selection = [m_pdfLayerController currentSelection];
</span><span class="cx"> if (existingSelectionContainsPoint(locationInViewCoordinates))
</span><del>- return [selection string];
</del><ins>+ return selection.string;
</ins><span class="cx">
</span><del>- IntPoint pointInView = convertFromPluginToPDFView(roundedIntPoint(locationInViewCoordinates));
- selection = [m_pdfLayerController getSelectionForWordAtPoint:pointInView];
</del><ins>+ IntPoint pointInPDFView = convertFromPluginToPDFView(convertFromRootViewToPlugin(roundedIntPoint(locationInViewCoordinates)));
+ selection = [m_pdfLayerController getSelectionForWordAtPoint:pointInPDFView];
</ins><span class="cx"> if (!selection)
</span><span class="cx"> return @"";
</span><span class="cx">
</span><del>- NSPoint pointInLayoutSpace = pointInLayoutSpaceForPointInWindowSpace(m_pdfLayerController.get(), pointInView);
</del><ins>+ NSPoint pointInLayoutSpace = pointInLayoutSpaceForPointInWindowSpace(m_pdfLayerController.get(), pointInPDFView);
</ins><span class="cx">
</span><span class="cx"> PDFPage *currentPage = [[m_pdfLayerController layout] pageNearestPoint:pointInLayoutSpace currentPage:[m_pdfLayerController currentPage]];
</span><span class="cx">
</span><span class="cx"> NSPoint pointInPageSpace = [[m_pdfLayerController layout] convertPoint:pointInLayoutSpace toPage:currentPage forScaleFactor:1.0];
</span><span class="cx">
</span><del>- for (PDFAnnotation *annotation in [currentPage annotations]) {
</del><ins>+ for (PDFAnnotation *annotation in currentPage.annotations) {
</ins><span class="cx"> if (![annotation isKindOfClass:pdfAnnotationLinkClass()])
</span><span class="cx"> continue;
</span><span class="cx">
</span><del>- NSRect bounds = [annotation bounds];
</del><ins>+ NSRect bounds = annotation.bounds;
</ins><span class="cx"> if (!NSPointInRect(pointInPageSpace, bounds))
</span><span class="cx"> continue;
</span><span class="cx">
</span><span class="cx"> PDFAnnotationLink *linkAnnotation = (PDFAnnotationLink *)annotation;
</span><del>- NSURL *url = [linkAnnotation URL];
</del><ins>+ NSURL *url = linkAnnotation.URL;
</ins><span class="cx"> if (!url)
</span><span class="cx"> continue;
</span><span class="cx">
</span><del>- data.absoluteLinkURL = [url absoluteString];
- data.linkLabel = [selection string];
- return [selection string];
</del><ins>+ data.absoluteLinkURL = url.absoluteString;
+ data.linkLabel = selection.string;
+ return selection.string;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> NSString *lookupText = dictionaryLookupForPDFSelection(selection, options);
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPagemacWebPageMacmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm (183281 => 183282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm 2015-04-24 21:39:06 UTC (rev 183281)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm 2015-04-24 21:53:47 UTC (rev 183282)
</span><span class="lines">@@ -1166,7 +1166,7 @@
</span><span class="cx"> // FIXME: We don't have API to identify images inside PDFs based on position.
</span><span class="cx"> NSDictionary *options = nil;
</span><span class="cx"> PDFSelection *selection = nil;
</span><del>- String selectedText = pdfPugin->lookupTextAtLocation(locationInContentCoordinates, actionMenuResult, &selection, &options);
</del><ins>+ String selectedText = pdfPugin->lookupTextAtLocation(locationInViewCoordinates, actionMenuResult, &selection, &options);
</ins><span class="cx"> if (!selectedText.isEmpty()) {
</span><span class="cx"> if (element->document().isPluginDocument()) {
</span><span class="cx"> // FIXME(144030): Focus does not seem to get set to the PDF when invoking the menu.
</span></span></pre></div>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (183281 => 183282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog 2015-04-24 21:39:06 UTC (rev 183281)
+++ trunk/Tools/ChangeLog 2015-04-24 21:53:47 UTC (rev 183282)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2015-04-24 Brent Fulgham <bfulgham@apple.com>
+
+ REGRESSION: WebKit2.ActionMenusTest API test fails
+ https://bugs.webkit.org/show_bug.cgi?id=144149
+ <rdar://problem/20677770>
+
+ Reviewed by Tim Horton.
+
+ * TestWebKitAPI/Tests/WebKit2/action-menu-targets.html: Revise to use the same test PDF
+ as the 'ActionMenusPDFTest'.
+ * TestWebKitAPI/Tests/WebKit2ObjC/ActionMenus.mm:
+ (TestWebKitAPI::TEST): Unskip the PDF portion of ActionMenusTest, and revise its expected output
+ to match the hit test on the 'action-menu-target.pdf' test file.
+
</ins><span class="cx"> 2015-04-24 Daniel Bates <dabates@apple.com>
</span><span class="cx">
</span><span class="cx"> Cleanup: Use @memoized for property IOSSimulator.testing_device
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsWebKit2actionmenutargetshtml"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2/action-menu-targets.html (183281 => 183282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/Tests/WebKit2/action-menu-targets.html 2015-04-24 21:39:06 UTC (rev 183281)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2/action-menu-targets.html 2015-04-24 21:53:47 UTC (rev 183282)
</span><span class="lines">@@ -110,7 +110,7 @@
</span><span class="cx"> <div style="top: 250px; left: 0px;"><img src="icon.png" height="100%"></div><br/>
</span><span class="cx"> <div style="top: 250px; left: 200px;"><a href="http://example.org/"><img src="icon.png" height="100%"></a></div><br/>
</span><span class="cx">
</span><del>-<div style="top: 250px; left: 400px; height: 350px"><embed src="test.pdf"></embed></div><br/>
</del><ins>+<div style="top: 250px; left: 400px; height: 350px"><embed src="action-menu-target.pdf"></embed></div><br/>
</ins><span class="cx">
</span><span class="cx"> <div style="top: 300px; left: 0px; width: 95px;"><a href="http://example.org/">http</a></div><br/>
</span><span class="cx"> <div style="top: 300px; left: 100px; width: 95px;"><a href="ftp://example.org/">ftp</a></div><br/>
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsWebKit2ObjCActionMenusmm"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2ObjC/ActionMenus.mm (183281 => 183282)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/Tests/WebKit2ObjC/ActionMenus.mm 2015-04-24 21:39:06 UTC (rev 183281)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2ObjC/ActionMenus.mm 2015-04-24 21:53:47 UTC (rev 183282)
</span><span class="lines">@@ -438,7 +438,7 @@
</span><span class="cx"> contentPoint = NSMakePoint(200, 350);
</span><span class="cx"> break;
</span><span class="cx"> case TargetType::PDFEmbed:
</span><del>- contentPoint = NSMakePoint(410, 420);
</del><ins>+ contentPoint = NSMakePoint(522, 363);
</ins><span class="cx"> break;
</span><span class="cx"> case TargetType::PDFDocument:
</span><span class="cx"> contentPoint = NSMakePoint(141, 374);
</span><span class="lines">@@ -734,14 +734,13 @@
</span><span class="cx"> EXPECT_FALSE(callJavaScriptReturningBool([wkView pageRef], "wasFailCalled()"));
</span><span class="cx"> }];
</span><span class="cx">
</span><del>- // FIXME(144149): Hit testing for embedded PDFs are not finding the right content. Temporarily skipping.
</del><span class="cx"> // PDF text content
</span><del>- //[wkView runMenuSequenceAtPoint:windowPointForTarget(TargetType::PDFEmbed) preDidCloseMenuHandler:^() {
- // EXPECT_EQ(kWKActionMenuReadOnlyText, [wkView _actionMenuResult].type);
- // EXPECT_WK_STREQ("Test", WKHitTestResultCopyLookupText([wkView _actionMenuResult].hitTestResult.get()));
- //
- // // FIXME(144008): You cannot copy from PDFs hosted in <embed> tags. When this is fixed, we should test it works here.
- //}];
</del><ins>+ [wkView runMenuSequenceAtPoint:windowPointForTarget(TargetType::PDFEmbed) preDidCloseMenuHandler:^() {
+ EXPECT_EQ(kWKActionMenuReadOnlyText, [wkView _actionMenuResult].type);
+ EXPECT_WK_STREQ("separation", WKHitTestResultCopyLookupText([wkView _actionMenuResult].hitTestResult.get()));
+
+ // FIXME(144008): You cannot copy from PDFs hosted in <embed> tags. When this is fixed, we should test it works here.
+ }];
</ins><span class="cx">
</span><span class="cx"> // Clients should be able to customize the menu by overriding WKView's _actionMenuItemsForHitTestResult.
</span><span class="cx"> // http://trac.webkit.org/changeset/174908
</span></span></pre>
</div>
</div>
</body>
</html>