PDF action menu fixes
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Apr 2015 02:43:40 +0000 (02:43 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Apr 2015 02:43:40 +0000 (02:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144299
<rdar://problem/20702215>

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
Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm
Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm

index 0a32334..459b1ac 100644 (file)
@@ -1,3 +1,29 @@
+2015-04-27  Brent Fulgham  <bfulgham@apple.com>
+
+        PDF action menu fixes
+        https://bugs.webkit.org/show_bug.cgi?id=144299
+        <rdar://problem/20702215>
+
+        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  <mcatanzaro@igalia.com>
 
         Rename WTF_USE_3D_GRAPHICS to ENABLE_GRAPHICS_CONTEXT_3D
index 9ddfb83..9487732 100644 (file)
@@ -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];
index 6fdb2d6..9748491 100644 (file)
@@ -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<NSMutableDictionary> 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;