<!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>[183447] 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/183447">183447</a></dd>
<dt>Author</dt> <dd>bfulgham@apple.com</dd>
<dt>Date</dt> <dd>2015-04-27 19:43:40 -0700 (Mon, 27 Apr 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2WebProcessPluginsPDFPDFPluginmm">trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm</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 (183446 => 183447)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2015-04-28 02:14:26 UTC (rev 183446)
+++ trunk/Source/WebKit2/ChangeLog 2015-04-28 02:43:40 UTC (rev 183447)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+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.
+
</ins><span class="cx"> 2015-04-27 Michael Catanzaro <mcatanzaro@igalia.com>
</span><span class="cx">
</span><span class="cx"> Rename WTF_USE_3D_GRAPHICS to ENABLE_GRAPHICS_CONTEXT_3D
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessPluginsPDFPDFPluginmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm (183446 => 183447)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm 2015-04-28 02:14:26 UTC (rev 183446)
+++ trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm 2015-04-28 02:43:40 UTC (rev 183447)
</span><span class="lines">@@ -1957,7 +1957,13 @@
</span><span class="cx">
</span><span class="cx"> WebCore::FloatRect PDFPlugin::viewRectForSelection(PDFSelection *selection) const
</span><span class="cx"> {
</span><del>- PDFPage *currentPage = [m_pdfLayerController currentPage];
</del><ins>+ PDFPage *currentPage = nil;
+ NSArray* pages = selection.pages;
+ if (pages.count)
+ currentPage = (PDFPage *)[pages objectAtIndex:0];
+
+ if (!currentPage)
+ currentPage = [m_pdfLayerController currentPage];
</ins><span class="cx">
</span><span class="cx"> NSRect rectInPageSpace = [selection boundsForPage:currentPage];
</span><span class="cx"> NSRect rectInLayoutSpace = [[m_pdfLayerController layout] convertRect:rectInPageSpace fromPage:currentPage forScaleFactor:1.0];
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPagemacWebPageMacmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm (183446 => 183447)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm 2015-04-28 02:14:26 UTC (rev 183446)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm 2015-04-28 02:43:40 UTC (rev 183447)
</span><span class="lines">@@ -605,12 +605,14 @@
</span><span class="cx"> CGFloat scaleFactor = pdfPlugin.scaleFactor();
</span><span class="cx">
</span><span class="cx"> __block CGFloat maxAscender = 0;
</span><ins>+ __block CGFloat maxDescender = 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<NSMutableDictionary> scaledAttributes = adoptNS([attributes mutableCopy]);
</span><span class="cx">
</span><span class="cx"> NSFont *font = [scaledAttributes objectForKey:NSFontAttributeName];
</span><span class="cx"> if (font) {
</span><span class="cx"> maxAscender = std::max(maxAscender, font.ascender * scaleFactor);
</span><ins>+ maxDescender = std::min(maxDescender, font.descender * scaleFactor);
</ins><span class="cx"> font = [fontManager convertFont:font toSize:[font pointSize] * scaleFactor];
</span><span class="cx"> [scaledAttributes setObject:font forKey:NSFontAttributeName];
</span><span class="cx"> }
</span><span class="lines">@@ -618,14 +620,17 @@
</span><span class="cx"> [scaledNSAttributedString addAttributes:scaledAttributes.get() range:range];
</span><span class="cx"> }];
</span><span class="cx">
</span><del>- CGFloat textInset = rangeRect.size.height - maxAscender;
- rangeRect.origin.y -= textInset;
</del><ins>+ // 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);
</ins><span class="cx">
</span><span class="cx"> TextIndicatorData dataForSelection;
</span><span class="cx"> dataForSelection.selectionRectInRootViewCoordinates = rangeRect;
</span><del>-
- CGFloat insetAmount = 0.5 * textInset;
- dataForSelection.textBoundingRectInRootViewCoordinates = NSInsetRect(rangeRect, insetAmount, insetAmount);
</del><ins>+ dataForSelection.textBoundingRectInRootViewCoordinates = rangeRect;
</ins><span class="cx"> dataForSelection.contentImageScaleFactor = scaleFactor;
</span><span class="cx"> dataForSelection.presentationTransition = presentationTransition;
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>