<!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>[183297] trunk/Source/WebKit2</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/183297">183297</a></dd>
<dt>Author</dt> <dd>bfulgham@apple.com</dd>
<dt>Date</dt> <dd>2015-04-24 17:16:52 -0700 (Fri, 24 Apr 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>TextIndicator for embedded PDFs is slightly offset
https://bugs.webkit.org/show_bug.cgi?id=144172
&lt;rdar://problem/20691304&gt;

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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2WebProcessPluginsPDFPDFPluginh">trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h</a></li>
<li><a href="#trunkSourceWebKit2WebProcessPluginsPDFPDFPluginmm">trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebPageWebPageh">trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebPagemacWebPageMacmm">trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (183296 => 183297)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,26 @@
</span><ins>+2015-04-24  Brent Fulgham  &lt;bfulgham@apple.com&gt;
+
+        TextIndicator for embedded PDFs is slightly offset
+        https://bugs.webkit.org/show_bug.cgi?id=144172
+        &lt;rdar://problem/20691304&gt;
+
+        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.
+
</ins><span class="cx"> 2015-04-24  David Kilzer  &lt;ddkilzer@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         REGRESSION (r183293): Fix iOS EWS build by adding SPI declaration for +[UIPeripheralHost visiblePeripheralFrame]
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessPluginsPDFPDFPluginh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h (183296 => 183297)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -107,6 +107,8 @@
</span><span class="cx">     String lookupTextAtLocation(const WebCore::FloatPoint&amp;, WebHitTestResult::Data&amp;, PDFSelection **, NSDictionary **) const;
</span><span class="cx">     WebCore::FloatRect viewRectForSelection(PDFSelection *) const;
</span><span class="cx"> 
</span><ins>+    CGFloat scaleFactor() const;
+
</ins><span class="cx"> private:
</span><span class="cx">     explicit PDFPlugin(WebFrame*);
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessPluginsPDFPDFPluginmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm (183296 => 183297)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1967,6 +1967,11 @@
</span><span class="cx">     return WebCore::FloatRect(rectInView);
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+CGFloat PDFPlugin::scaleFactor() const
+{
+    return [m_pdfLayerController contentScaleFactor];
+}
+
</ins><span class="cx"> void PDFPlugin::performWebSearch(NSString *string)
</span><span class="cx"> {
</span><span class="cx">     webFrame()-&gt;page()-&gt;send(Messages::WebPageProxy::SearchTheWeb(string));
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPageWebPageh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h (183296 => 183297)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1015,7 +1015,7 @@
</span><span class="cx">     void performDictionaryLookupForRange(WebCore::Frame*, WebCore::Range&amp;, NSDictionary *options, WebCore::TextIndicatorPresentationTransition);
</span><span class="cx">     DictionaryPopupInfo dictionaryPopupInfoForRange(WebCore::Frame* frame, WebCore::Range&amp; range, NSDictionary **options, WebCore::TextIndicatorPresentationTransition presentationTransition);
</span><span class="cx"> #if ENABLE(PDFKIT_PLUGIN)
</span><del>-    DictionaryPopupInfo dictionaryPopupInfoForPDFSelectionInPluginView(PDFSelection *, PDFPlugin&amp;, NSDictionary **options, WebCore::TextIndicatorPresentationTransition);
</del><ins>+    DictionaryPopupInfo dictionaryPopupInfoForSelectionInPDFPlugin(PDFSelection *, PDFPlugin&amp;, NSDictionary **options, WebCore::TextIndicatorPresentationTransition);
</ins><span class="cx"> #endif
</span><span class="cx"> 
</span><span class="cx">     void windowAndViewFramesChanged(const WebCore::FloatRect&amp; windowFrameInScreenCoordinates, const WebCore::FloatRect&amp; windowFrameInUnflippedScreenCoordinates, const WebCore::FloatRect&amp; viewFrameInWindowCoordinates, const WebCore::FloatPoint&amp; accessibilityViewCoordinates);
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPagemacWebPageMacmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm (183296 => 183297)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -588,7 +588,7 @@
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> #if ENABLE(PDFKIT_PLUGIN)
</span><del>-DictionaryPopupInfo WebPage::dictionaryPopupInfoForPDFSelectionInPluginView(PDFSelection *selection, PDFPlugin&amp; pdfPlugin, NSDictionary **options, WebCore::TextIndicatorPresentationTransition presentationTransition)
</del><ins>+DictionaryPopupInfo WebPage::dictionaryPopupInfoForSelectionInPDFPlugin(PDFSelection *selection, PDFPlugin&amp; pdfPlugin, NSDictionary **options, WebCore::TextIndicatorPresentationTransition presentationTransition)
</ins><span class="cx"> {
</span><span class="cx">     DictionaryPopupInfo dictionaryPopupInfo;
</span><span class="cx">     if (!selection.string.length)
</span><span class="lines">@@ -596,33 +596,41 @@
</span><span class="cx"> 
</span><span class="cx">     NSRect rangeRect = pdfPlugin.viewRectForSelection(selection);
</span><span class="cx"> 
</span><del>-    dictionaryPopupInfo.origin = rangeRect.origin;
-    dictionaryPopupInfo.options = (CFDictionaryRef)*options;
-    
</del><span class="cx">     NSAttributedString *nsAttributedString = selection.attributedString;
</span><span class="cx">     
</span><span class="cx">     RetainPtr&lt;NSMutableAttributedString&gt; scaledNSAttributedString = adoptNS([[NSMutableAttributedString alloc] initWithString:[nsAttributedString string]]);
</span><span class="cx">     
</span><span class="cx">     NSFontManager *fontManager = [NSFontManager sharedFontManager];
</span><del>-    
</del><ins>+
+    CGFloat scaleFactor = pdfPlugin.scaleFactor();
+
+    __block CGFloat maxAscender = 0;
</ins><span class="cx">     [nsAttributedString enumerateAttributesInRange:NSMakeRange(0, [nsAttributedString length]) options:0 usingBlock:^(NSDictionary *attributes, NSRange range, BOOL *stop) {
</span><span class="cx">         RetainPtr&lt;NSMutableDictionary&gt; scaledAttributes = adoptNS([attributes mutableCopy]);
</span><span class="cx">         
</span><span class="cx">         NSFont *font = [scaledAttributes objectForKey:NSFontAttributeName];
</span><span class="cx">         if (font) {
</span><del>-            font = [fontManager convertFont:font toSize:[font pointSize] * pageScaleFactor()];
</del><ins>+            maxAscender = std::max(maxAscender, font.ascender * scaleFactor);
+            font = [fontManager convertFont:font toSize:[font pointSize] * scaleFactor];
</ins><span class="cx">             [scaledAttributes setObject:font forKey:NSFontAttributeName];
</span><span class="cx">         }
</span><span class="cx">         
</span><span class="cx">         [scaledNSAttributedString addAttributes:scaledAttributes.get() range:range];
</span><span class="cx">     }];
</span><span class="cx"> 
</span><ins>+    CGFloat textInset = rangeRect.size.height - maxAscender;
+    rangeRect.origin.y -= textInset;
+    
</ins><span class="cx">     TextIndicatorData dataForSelection;
</span><span class="cx">     dataForSelection.selectionRectInRootViewCoordinates = rangeRect;
</span><del>-    dataForSelection.textBoundingRectInRootViewCoordinates = rangeRect;
-    dataForSelection.contentImageScaleFactor = 1.0;
</del><ins>+
+    CGFloat insetAmount = 0.5 * textInset;
+    dataForSelection.textBoundingRectInRootViewCoordinates = NSInsetRect(rangeRect, insetAmount, insetAmount);
+    dataForSelection.contentImageScaleFactor = scaleFactor;
</ins><span class="cx">     dataForSelection.presentationTransition = presentationTransition;
</span><span class="cx">     
</span><ins>+    dictionaryPopupInfo.origin = rangeRect.origin;
+    dictionaryPopupInfo.options = (CFDictionaryRef)*options;
</ins><span class="cx">     dictionaryPopupInfo.textIndicator = dataForSelection;
</span><span class="cx">     dictionaryPopupInfo.attributedString.string = scaledNSAttributedString;
</span><span class="cx">     
</span><span class="lines">@@ -1179,7 +1187,7 @@
</span><span class="cx">                 actionMenuResult.isSelected = true;
</span><span class="cx">                 actionMenuResult.allowsCopy = true;
</span><span class="cx"> 
</span><del>-                actionMenuResult.dictionaryPopupInfo = dictionaryPopupInfoForPDFSelectionInPluginView(selection, *pdfPugin, &amp;options, textIndicatorTransitionForActionMenu(forImmediateAction, false));
</del><ins>+                actionMenuResult.dictionaryPopupInfo = dictionaryPopupInfoForSelectionInPDFPlugin(selection, *pdfPugin, &amp;options, textIndicatorTransitionForActionMenu(forImmediateAction, false));
</ins><span class="cx">             }
</span><span class="cx">         }
</span><span class="cx">     }
</span></span></pre>
</div>
</div>

</body>
</html>