- Revision
- 183297
- Author
- [email protected]
- Date
- 2015-04-24 17:16:52 -0700 (Fri, 24 Apr 2015)
Log Message
TextIndicator for embedded PDFs is slightly offset
https://bugs.webkit.org/show_bug.cgi?id=144172
<rdar://problem/20691304>
Reviewed by Tim Horton.
When I converted the existing DOM Range logic to work with PDFSelections, I omitted the
step where the font ascent was used to adjust the origin used for the TextIndicator. This
patch determines the correct ascent for the range of characters in the selection, and
adjusts the offset by the difference between the ascent and the height of the selection rect.
Also, since the PDFSelection only supplies the bounding rect for the selection, I calculate
an equivalent text bounding box by insetting the rect by half the size of the ascent.
* WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::scaleFactor): Add accessor for PDF scale factor.
* WebProcess/Plugins/PDF/PDFPlugin.h:
* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::dictionaryPopupInfoForPDFSelectionInPluginView): Adjusted to take the
font ascent and scale factor into account.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (183296 => 183297)
--- trunk/Source/WebKit2/ChangeLog 2015-04-25 00:11:43 UTC (rev 183296)
+++ trunk/Source/WebKit2/ChangeLog 2015-04-25 00:16:52 UTC (rev 183297)
@@ -1,3 +1,26 @@
+2015-04-24 Brent Fulgham <[email protected]>
+
+ TextIndicator for embedded PDFs is slightly offset
+ https://bugs.webkit.org/show_bug.cgi?id=144172
+ <rdar://problem/20691304>
+
+ Reviewed by Tim Horton.
+
+ When I converted the existing DOM Range logic to work with PDFSelections, I omitted the
+ step where the font ascent was used to adjust the origin used for the TextIndicator. This
+ patch determines the correct ascent for the range of characters in the selection, and
+ adjusts the offset by the difference between the ascent and the height of the selection rect.
+
+ Also, since the PDFSelection only supplies the bounding rect for the selection, I calculate
+ an equivalent text bounding box by insetting the rect by half the size of the ascent.
+
+ * WebProcess/Plugins/PDF/PDFPlugin.mm:
+ (WebKit::PDFPlugin::scaleFactor): Add accessor for PDF scale factor.
+ * WebProcess/Plugins/PDF/PDFPlugin.h:
+ * WebProcess/WebPage/mac/WebPageMac.mm:
+ (WebKit::WebPage::dictionaryPopupInfoForPDFSelectionInPluginView): Adjusted to take the
+ font ascent and scale factor into account.
+
2015-04-24 David Kilzer <[email protected]>
REGRESSION (r183293): Fix iOS EWS build by adding SPI declaration for +[UIPeripheralHost visiblePeripheralFrame]
Modified: trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h (183296 => 183297)
--- trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h 2015-04-25 00:11:43 UTC (rev 183296)
+++ trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h 2015-04-25 00:16:52 UTC (rev 183297)
@@ -107,6 +107,8 @@
String lookupTextAtLocation(const WebCore::FloatPoint&, WebHitTestResult::Data&, PDFSelection **, NSDictionary **) const;
WebCore::FloatRect viewRectForSelection(PDFSelection *) const;
+ CGFloat scaleFactor() const;
+
private:
explicit PDFPlugin(WebFrame*);
Modified: trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm (183296 => 183297)
--- trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm 2015-04-25 00:11:43 UTC (rev 183296)
+++ trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm 2015-04-25 00:16:52 UTC (rev 183297)
@@ -1967,6 +1967,11 @@
return WebCore::FloatRect(rectInView);
}
+CGFloat PDFPlugin::scaleFactor() const
+{
+ return [m_pdfLayerController contentScaleFactor];
+}
+
void PDFPlugin::performWebSearch(NSString *string)
{
webFrame()->page()->send(Messages::WebPageProxy::SearchTheWeb(string));
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h (183296 => 183297)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h 2015-04-25 00:11:43 UTC (rev 183296)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h 2015-04-25 00:16:52 UTC (rev 183297)
@@ -1015,7 +1015,7 @@
void performDictionaryLookupForRange(WebCore::Frame*, WebCore::Range&, NSDictionary *options, WebCore::TextIndicatorPresentationTransition);
DictionaryPopupInfo dictionaryPopupInfoForRange(WebCore::Frame* frame, WebCore::Range& range, NSDictionary **options, WebCore::TextIndicatorPresentationTransition presentationTransition);
#if ENABLE(PDFKIT_PLUGIN)
- DictionaryPopupInfo dictionaryPopupInfoForPDFSelectionInPluginView(PDFSelection *, PDFPlugin&, NSDictionary **options, WebCore::TextIndicatorPresentationTransition);
+ DictionaryPopupInfo dictionaryPopupInfoForSelectionInPDFPlugin(PDFSelection *, PDFPlugin&, NSDictionary **options, WebCore::TextIndicatorPresentationTransition);
#endif
void windowAndViewFramesChanged(const WebCore::FloatRect& windowFrameInScreenCoordinates, const WebCore::FloatRect& windowFrameInUnflippedScreenCoordinates, const WebCore::FloatRect& viewFrameInWindowCoordinates, const WebCore::FloatPoint& accessibilityViewCoordinates);
Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm (183296 => 183297)
--- trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm 2015-04-25 00:11:43 UTC (rev 183296)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm 2015-04-25 00:16:52 UTC (rev 183297)
@@ -588,7 +588,7 @@
}
#if ENABLE(PDFKIT_PLUGIN)
-DictionaryPopupInfo WebPage::dictionaryPopupInfoForPDFSelectionInPluginView(PDFSelection *selection, PDFPlugin& pdfPlugin, NSDictionary **options, WebCore::TextIndicatorPresentationTransition presentationTransition)
+DictionaryPopupInfo WebPage::dictionaryPopupInfoForSelectionInPDFPlugin(PDFSelection *selection, PDFPlugin& pdfPlugin, NSDictionary **options, WebCore::TextIndicatorPresentationTransition presentationTransition)
{
DictionaryPopupInfo dictionaryPopupInfo;
if (!selection.string.length)
@@ -596,33 +596,41 @@
NSRect rangeRect = pdfPlugin.viewRectForSelection(selection);
- dictionaryPopupInfo.origin = rangeRect.origin;
- dictionaryPopupInfo.options = (CFDictionaryRef)*options;
-
NSAttributedString *nsAttributedString = selection.attributedString;
RetainPtr<NSMutableAttributedString> scaledNSAttributedString = adoptNS([[NSMutableAttributedString alloc] initWithString:[nsAttributedString string]]);
NSFontManager *fontManager = [NSFontManager sharedFontManager];
-
+
+ CGFloat scaleFactor = pdfPlugin.scaleFactor();
+
+ __block CGFloat maxAscender = 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) {
- font = [fontManager convertFont:font toSize:[font pointSize] * pageScaleFactor()];
+ maxAscender = std::max(maxAscender, font.ascender * scaleFactor);
+ font = [fontManager convertFont:font toSize:[font pointSize] * scaleFactor];
[scaledAttributes setObject:font forKey:NSFontAttributeName];
}
[scaledNSAttributedString addAttributes:scaledAttributes.get() range:range];
}];
+ CGFloat textInset = rangeRect.size.height - maxAscender;
+ rangeRect.origin.y -= textInset;
+
TextIndicatorData dataForSelection;
dataForSelection.selectionRectInRootViewCoordinates = rangeRect;
- dataForSelection.textBoundingRectInRootViewCoordinates = rangeRect;
- dataForSelection.contentImageScaleFactor = 1.0;
+
+ CGFloat insetAmount = 0.5 * textInset;
+ dataForSelection.textBoundingRectInRootViewCoordinates = NSInsetRect(rangeRect, insetAmount, insetAmount);
+ dataForSelection.contentImageScaleFactor = scaleFactor;
dataForSelection.presentationTransition = presentationTransition;
+ dictionaryPopupInfo.origin = rangeRect.origin;
+ dictionaryPopupInfo.options = (CFDictionaryRef)*options;
dictionaryPopupInfo.textIndicator = dataForSelection;
dictionaryPopupInfo.attributedString.string = scaledNSAttributedString;
@@ -1179,7 +1187,7 @@
actionMenuResult.isSelected = true;
actionMenuResult.allowsCopy = true;
- actionMenuResult.dictionaryPopupInfo = dictionaryPopupInfoForPDFSelectionInPluginView(selection, *pdfPugin, &options, textIndicatorTransitionForActionMenu(forImmediateAction, false));
+ actionMenuResult.dictionaryPopupInfo = dictionaryPopupInfoForSelectionInPDFPlugin(selection, *pdfPugin, &options, textIndicatorTransitionForActionMenu(forImmediateAction, false));
}
}
}
