From db14690794f16351e27a49b2064c54e5b09baf52 Mon Sep 17 00:00:00 2001 From: "bfulgham@apple.com" Date: Tue, 28 Apr 2015 02:43:40 +0000 Subject: [PATCH] PDF action menu fixes https://bugs.webkit.org/show_bug.cgi?id=144299 Reviewed by Tim Horton. Make two corrections to how PDFs are handled: 1. When calculating the view rect for the user's selection, make sure that we get coordinates for the correct PDF page. The existing code assumed that the current PDFLayerControler's current page was correct, but this will not be true if you zoom the PDF out so that several pages are displayed at once. Each selection keeps track of the page it is referenced against. 2. Revise the offsets calculated for the TextIndicator to take into account the font descender (as well as the ascender), and to adjust by the scaled amount of margin around the selected text. * WebProcess/Plugins/PDF/PDFPlugin.mm: (WebKit::PDFPlugin::viewRectForSelection): Use correct page for calculating the coordinates. * WebProcess/WebPage/mac/WebPageMac.mm: (WebKit::WebPage::dictionaryPopupInfoForSelectionInPDFPlugin): Include font 'descendant' and (scaled) margin when adjusting the hit target for the TextIndicator to draw. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@183447 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebKit2/ChangeLog | 26 ++++++++++++++++++++++ Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm | 8 ++++++- .../WebKit2/WebProcess/WebPage/mac/WebPageMac.mm | 15 ++++++++----- 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog index 0a32334..459b1ac 100644 --- a/Source/WebKit2/ChangeLog +++ b/Source/WebKit2/ChangeLog @@ -1,3 +1,29 @@ +2015-04-27 Brent Fulgham + + PDF action menu fixes + https://bugs.webkit.org/show_bug.cgi?id=144299 + + + Reviewed by Tim Horton. + + Make two corrections to how PDFs are handled: + 1. When calculating the view rect for the user's selection, make sure + that we get coordinates for the correct PDF page. The existing code assumed + that the current PDFLayerControler's current page was correct, but this will + not be true if you zoom the PDF out so that several pages are displayed at + once. Each selection keeps track of the page it is referenced against. + + 2. Revise the offsets calculated for the TextIndicator to take into account + the font descender (as well as the ascender), and to adjust by the scaled + amount of margin around the selected text. + + * WebProcess/Plugins/PDF/PDFPlugin.mm: + (WebKit::PDFPlugin::viewRectForSelection): Use correct page for calculating + the coordinates. + * WebProcess/WebPage/mac/WebPageMac.mm: + (WebKit::WebPage::dictionaryPopupInfoForSelectionInPDFPlugin): Include font 'descendant' + and (scaled) margin when adjusting the hit target for the TextIndicator to draw. + 2015-04-27 Michael Catanzaro Rename WTF_USE_3D_GRAPHICS to ENABLE_GRAPHICS_CONTEXT_3D diff --git a/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm b/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm index 9ddfb83..9487732 100644 --- a/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm +++ b/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm @@ -1957,7 +1957,13 @@ static NSRect rectInViewSpaceForRectInLayoutSpace(PDFLayerController* pdfLayerCo WebCore::FloatRect PDFPlugin::viewRectForSelection(PDFSelection *selection) const { - PDFPage *currentPage = [m_pdfLayerController currentPage]; + PDFPage *currentPage = nil; + NSArray* pages = selection.pages; + if (pages.count) + currentPage = (PDFPage *)[pages objectAtIndex:0]; + + if (!currentPage) + currentPage = [m_pdfLayerController currentPage]; NSRect rectInPageSpace = [selection boundsForPage:currentPage]; NSRect rectInLayoutSpace = [[m_pdfLayerController layout] convertRect:rectInPageSpace fromPage:currentPage forScaleFactor:1.0]; diff --git a/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm b/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm index 6fdb2d6..9748491 100644 --- a/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm +++ b/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm @@ -605,12 +605,14 @@ DictionaryPopupInfo WebPage::dictionaryPopupInfoForSelectionInPDFPlugin(PDFSelec CGFloat scaleFactor = pdfPlugin.scaleFactor(); __block CGFloat maxAscender = 0; + __block CGFloat maxDescender = 0; [nsAttributedString enumerateAttributesInRange:NSMakeRange(0, [nsAttributedString length]) options:0 usingBlock:^(NSDictionary *attributes, NSRange range, BOOL *stop) { RetainPtr scaledAttributes = adoptNS([attributes mutableCopy]); NSFont *font = [scaledAttributes objectForKey:NSFontAttributeName]; if (font) { maxAscender = std::max(maxAscender, font.ascender * scaleFactor); + maxDescender = std::min(maxDescender, font.descender * scaleFactor); font = [fontManager convertFont:font toSize:[font pointSize] * scaleFactor]; [scaledAttributes setObject:font forKey:NSFontAttributeName]; } @@ -618,14 +620,17 @@ DictionaryPopupInfo WebPage::dictionaryPopupInfoForSelectionInPDFPlugin(PDFSelec [scaledNSAttributedString addAttributes:scaledAttributes.get() range:range]; }]; - CGFloat textInset = rangeRect.size.height - maxAscender; - rangeRect.origin.y -= textInset; + // Based on TextIndicator implementation: + // TODO(144307): Come up with a better way to share this information than duplicating these values. + CGFloat verticalMargin = 2.5; + CGFloat horizontalMargin = 0.5; + + rangeRect.origin.y -= CGCeiling(rangeRect.size.height - maxAscender - std::abs(maxDescender) + verticalMargin * scaleFactor); + rangeRect.origin.x += CGFloor(horizontalMargin * scaleFactor); TextIndicatorData dataForSelection; dataForSelection.selectionRectInRootViewCoordinates = rangeRect; - - CGFloat insetAmount = 0.5 * textInset; - dataForSelection.textBoundingRectInRootViewCoordinates = NSInsetRect(rangeRect, insetAmount, insetAmount); + dataForSelection.textBoundingRectInRootViewCoordinates = rangeRect; dataForSelection.contentImageScaleFactor = scaleFactor; dataForSelection.presentationTransition = presentationTransition; -- 1.8.3.1