- Revision
- 183282
- Author
- [email protected]
- Date
- 2015-04-24 14:53:47 -0700 (Fri, 24 Apr 2015)
Log Message
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.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (183281 => 183282)
--- 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)
@@ -1,3 +1,30 @@
+2015-04-24 Brent Fulgham <[email protected]>
+
+ 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.
+
2015-04-24 Anders Carlsson <[email protected]>
Address a review comment from Joe Pecoraro.
Modified: trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm (183281 => 183282)
--- 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)
@@ -1846,14 +1846,14 @@
return [selectionForWord string];
}
-bool PDFPlugin::existingSelectionContainsPoint(const WebCore::FloatPoint& point) const
+bool PDFPlugin::existingSelectionContainsPoint(const WebCore::FloatPoint& locationInViewCoordinates) const
{
PDFSelection *currentSelection = [m_pdfLayerController currentSelection];
if (!currentSelection)
return false;
- IntPoint pointInView = convertFromPluginToPDFView(roundedIntPoint(point));
- PDFSelection *selectionForWord = [m_pdfLayerController getSelectionForWordAtPoint:pointInView];
+ IntPoint pointInPDFView = convertFromPluginToPDFView(convertFromRootViewToPlugin(roundedIntPoint(locationInViewCoordinates)));
+ PDFSelection *selectionForWord = [m_pdfLayerController getSelectionForWordAtPoint:pointInPDFView];
NSUInteger currentPageIndex = [m_pdfLayerController currentPageIndex];
@@ -1896,35 +1896,35 @@
selection = [m_pdfLayerController currentSelection];
if (existingSelectionContainsPoint(locationInViewCoordinates))
- return [selection string];
+ return selection.string;
- IntPoint pointInView = convertFromPluginToPDFView(roundedIntPoint(locationInViewCoordinates));
- selection = [m_pdfLayerController getSelectionForWordAtPoint:pointInView];
+ IntPoint pointInPDFView = convertFromPluginToPDFView(convertFromRootViewToPlugin(roundedIntPoint(locationInViewCoordinates)));
+ selection = [m_pdfLayerController getSelectionForWordAtPoint:pointInPDFView];
if (!selection)
return @"";
- NSPoint pointInLayoutSpace = pointInLayoutSpaceForPointInWindowSpace(m_pdfLayerController.get(), pointInView);
+ NSPoint pointInLayoutSpace = pointInLayoutSpaceForPointInWindowSpace(m_pdfLayerController.get(), pointInPDFView);
PDFPage *currentPage = [[m_pdfLayerController layout] pageNearestPoint:pointInLayoutSpace currentPage:[m_pdfLayerController currentPage]];
NSPoint pointInPageSpace = [[m_pdfLayerController layout] convertPoint:pointInLayoutSpace toPage:currentPage forScaleFactor:1.0];
- for (PDFAnnotation *annotation in [currentPage annotations]) {
+ for (PDFAnnotation *annotation in currentPage.annotations) {
if (![annotation isKindOfClass:pdfAnnotationLinkClass()])
continue;
- NSRect bounds = [annotation bounds];
+ NSRect bounds = annotation.bounds;
if (!NSPointInRect(pointInPageSpace, bounds))
continue;
PDFAnnotationLink *linkAnnotation = (PDFAnnotationLink *)annotation;
- NSURL *url = "" URL];
+ NSURL *url = ""
if (!url)
continue;
- data.absoluteLinkURL = [url absoluteString];
- data.linkLabel = [selection string];
- return [selection string];
+ data.absoluteLinkURL = url.absoluteString;
+ data.linkLabel = selection.string;
+ return selection.string;
}
NSString *lookupText = dictionaryLookupForPDFSelection(selection, options);
Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm (183281 => 183282)
--- 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)
@@ -1166,7 +1166,7 @@
// FIXME: We don't have API to identify images inside PDFs based on position.
NSDictionary *options = nil;
PDFSelection *selection = nil;
- String selectedText = pdfPugin->lookupTextAtLocation(locationInContentCoordinates, actionMenuResult, &selection, &options);
+ String selectedText = pdfPugin->lookupTextAtLocation(locationInViewCoordinates, actionMenuResult, &selection, &options);
if (!selectedText.isEmpty()) {
if (element->document().isPluginDocument()) {
// FIXME(144030): Focus does not seem to get set to the PDF when invoking the menu.
Modified: trunk/Tools/ChangeLog (183281 => 183282)
--- trunk/Tools/ChangeLog 2015-04-24 21:39:06 UTC (rev 183281)
+++ trunk/Tools/ChangeLog 2015-04-24 21:53:47 UTC (rev 183282)
@@ -1,3 +1,17 @@
+2015-04-24 Brent Fulgham <[email protected]>
+
+ 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.
+
2015-04-24 Daniel Bates <[email protected]>
Cleanup: Use @memoized for property IOSSimulator.testing_device
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2/action-menu-targets.html (183281 => 183282)
--- 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)
@@ -110,7 +110,7 @@
<div style="top: 250px; left: 0px;"><img src="" height="100%"></div><br/>
<div style="top: 250px; left: 200px;"><a href="" src="" height="100%"></a></div><br/>
-<div style="top: 250px; left: 400px; height: 350px"><embed src=""
+<div style="top: 250px; left: 400px; height: 350px"><embed src=""
<div style="top: 300px; left: 0px; width: 95px;"><a href=""
<div style="top: 300px; left: 100px; width: 95px;"><a href=""
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2ObjC/ActionMenus.mm (183281 => 183282)
--- 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)
@@ -438,7 +438,7 @@
contentPoint = NSMakePoint(200, 350);
break;
case TargetType::PDFEmbed:
- contentPoint = NSMakePoint(410, 420);
+ contentPoint = NSMakePoint(522, 363);
break;
case TargetType::PDFDocument:
contentPoint = NSMakePoint(141, 374);
@@ -734,14 +734,13 @@
EXPECT_FALSE(callJavaScriptReturningBool([wkView pageRef], "wasFailCalled()"));
}];
- // FIXME(144149): Hit testing for embedded PDFs are not finding the right content. Temporarily skipping.
// PDF text content
- //[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.
- //}];
+ [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.
+ }];
// Clients should be able to customize the menu by overriding WKView's _actionMenuItemsForHitTestResult.
// http://trac.webkit.org/changeset/174908
