<!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>[186978] trunk/Source</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/186978">186978</a></dd>
<dt>Author</dt> <dd>timothy_horton@apple.com</dd>
<dt>Date</dt> <dd>2015-07-17 17:20:35 -0700 (Fri, 17 Jul 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>[iOS] TextIndicator has a large forehead when line-height &gt; 1
https://bugs.webkit.org/show_bug.cgi?id=147058
&lt;rdar://problem/21643094&gt;

Reviewed by Dean Jackson.

* editing/FrameSelection.cpp:
(WebCore::FrameSelection::getClippedVisibleTextRectangles):
* editing/FrameSelection.h:
Add a parameter controlling whether getClippedVisibleTextRectangles
returns selection-height rects (including extra line-height) or text-height
rects (including only the text height). Plumb it down.

* page/TextIndicator.cpp:
(WebCore::TextIndicator::createWithRange):
(WebCore::TextIndicator::createWithSelectionInFrame):
Use the tighter text-height rects on iOS, where there's no selection highlight to cover up.
Remove an assertion that is no longer always true, and which is mostly obsoleted by the
fact that we don't let FrameSnapshotting code arbitrarily decide the rect to snapshot anymore.

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::getPositionInformation):
Apply a review comment that I left myself and then forgot about.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreeditingFrameSelectioncpp">trunk/Source/WebCore/editing/FrameSelection.cpp</a></li>
<li><a href="#trunkSourceWebCoreeditingFrameSelectionh">trunk/Source/WebCore/editing/FrameSelection.h</a></li>
<li><a href="#trunkSourceWebCorepageTextIndicatorcpp">trunk/Source/WebCore/page/TextIndicator.cpp</a></li>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebPageiosWebPageIOSmm">trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (186977 => 186978)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2015-07-17 23:59:12 UTC (rev 186977)
+++ trunk/Source/WebCore/ChangeLog        2015-07-18 00:20:35 UTC (rev 186978)
</span><span class="lines">@@ -1,5 +1,27 @@
</span><span class="cx"> 2015-07-17  Tim Horton  &lt;timothy_horton@apple.com&gt;
</span><span class="cx"> 
</span><ins>+        [iOS] TextIndicator has a large forehead when line-height &gt; 1
+        https://bugs.webkit.org/show_bug.cgi?id=147058
+        &lt;rdar://problem/21643094&gt;
+
+        Reviewed by Dean Jackson.
+
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::getClippedVisibleTextRectangles):
+        * editing/FrameSelection.h:
+        Add a parameter controlling whether getClippedVisibleTextRectangles
+        returns selection-height rects (including extra line-height) or text-height
+        rects (including only the text height). Plumb it down.
+
+        * page/TextIndicator.cpp:
+        (WebCore::TextIndicator::createWithRange):
+        (WebCore::TextIndicator::createWithSelectionInFrame):
+        Use the tighter text-height rects on iOS, where there's no selection highlight to cover up.
+        Remove an assertion that is no longer always true, and which is mostly obsoleted by the
+        fact that we don't let FrameSnapshotting code arbitrarily decide the rect to snapshot anymore.
+
+2015-07-17  Tim Horton  &lt;timothy_horton@apple.com&gt;
+
</ins><span class="cx">         Improve rect shrink-wrapping algorithm
</span><span class="cx">         https://bugs.webkit.org/show_bug.cgi?id=147037
</span><span class="cx">         &lt;rdar://problem/21643094&gt;
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingFrameSelectioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (186977 => 186978)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/editing/FrameSelection.cpp        2015-07-17 23:59:12 UTC (rev 186977)
+++ trunk/Source/WebCore/editing/FrameSelection.cpp        2015-07-18 00:20:35 UTC (rev 186978)
</span><span class="lines">@@ -2075,7 +2075,7 @@
</span><span class="cx">     return clipToVisibleContent ? intersection(selectionRect, view-&gt;visibleContentRect(ScrollableArea::LegacyIOSDocumentVisibleRect)) : selectionRect;
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void FrameSelection::getClippedVisibleTextRectangles(Vector&lt;FloatRect&gt;&amp; rectangles) const
</del><ins>+void FrameSelection::getClippedVisibleTextRectangles(Vector&lt;FloatRect&gt;&amp; rectangles, TextRectangleHeight textRectHeight) const
</ins><span class="cx"> {
</span><span class="cx">     RenderView* root = m_frame-&gt;contentRenderer();
</span><span class="cx">     if (!root)
</span><span class="lines">@@ -2084,7 +2084,7 @@
</span><span class="cx">     FloatRect visibleContentRect = m_frame-&gt;view()-&gt;visibleContentRect(ScrollableArea::LegacyIOSDocumentVisibleRect);
</span><span class="cx"> 
</span><span class="cx">     Vector&lt;FloatQuad&gt; quads;
</span><del>-    toNormalizedRange()-&gt;textQuads(quads, true);
</del><ins>+    toNormalizedRange()-&gt;textQuads(quads, textRectHeight == TextRectangleHeight::SelectionHeight);
</ins><span class="cx"> 
</span><span class="cx">     size_t size = quads.size();
</span><span class="cx">     for (size_t i = 0; i &lt; size; ++i) {
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingFrameSelectionh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/FrameSelection.h (186977 => 186978)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/editing/FrameSelection.h        2015-07-17 23:59:12 UTC (rev 186977)
+++ trunk/Source/WebCore/editing/FrameSelection.h        2015-07-18 00:20:35 UTC (rev 186978)
</span><span class="lines">@@ -260,7 +260,8 @@
</span><span class="cx"> 
</span><span class="cx">     WEBCORE_EXPORT FloatRect selectionBounds(bool clipToVisibleContent = true) const;
</span><span class="cx"> 
</span><del>-    WEBCORE_EXPORT void getClippedVisibleTextRectangles(Vector&lt;FloatRect&gt;&amp;) const;
</del><ins>+    enum class TextRectangleHeight { TextHeight, SelectionHeight };
+    WEBCORE_EXPORT void getClippedVisibleTextRectangles(Vector&lt;FloatRect&gt;&amp;, TextRectangleHeight = TextRectangleHeight::SelectionHeight) const;
</ins><span class="cx"> 
</span><span class="cx">     WEBCORE_EXPORT HTMLFormElement* currentForm() const;
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCorepageTextIndicatorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/TextIndicator.cpp (186977 => 186978)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/TextIndicator.cpp        2015-07-17 23:59:12 UTC (rev 186977)
+++ trunk/Source/WebCore/page/TextIndicator.cpp        2015-07-18 00:20:35 UTC (rev 186978)
</span><span class="lines">@@ -104,8 +104,17 @@
</span><span class="cx"> RefPtr&lt;TextIndicator&gt; TextIndicator::createWithSelectionInFrame(Frame&amp; frame, TextIndicatorPresentationTransition presentationTransition, unsigned margin)
</span><span class="cx"> {
</span><span class="cx">     Vector&lt;FloatRect&gt; textRects;
</span><del>-    frame.selection().getClippedVisibleTextRectangles(textRects);
</del><span class="cx"> 
</span><ins>+    // On iOS, we don't need to expand the TextIndicator to cover the whole selection height.
+    // FIXME: Ideally, on Mac, there are times when we don't need to (if we don't have a selection),
+    // and using TextHeight would provide a more sensible appearance.
+#if PLATFORM(IOS)
+    FrameSelection::TextRectangleHeight textRectHeight = FrameSelection::TextRectangleHeight::TextHeight;
+#else
+    FrameSelection::TextRectangleHeight textRectHeight = FrameSelection::TextRectangleHeight::SelectionHeight;
+#endif
+    frame.selection().getClippedVisibleTextRectangles(textRects, textRectHeight);
+
</ins><span class="cx">     // The bounding rect of all the text rects can be different than the selection
</span><span class="cx">     // rect when the selection spans multiple lines; the indicator doesn't actually
</span><span class="cx">     // care where the selection highlight goes, just where the text actually is.
</span><span class="lines">@@ -166,7 +175,6 @@
</span><span class="cx"> TextIndicator::TextIndicator(const TextIndicatorData&amp; data)
</span><span class="cx">     : m_data(data)
</span><span class="cx"> {
</span><del>-    ASSERT(m_data.contentImageScaleFactor != 1 || m_data.contentImage-&gt;size() == enclosingIntRect(m_data.selectionRectInRootViewCoordinates).size());
</del><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> TextIndicator::~TextIndicator()
</span></span></pre></div>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (186977 => 186978)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog        2015-07-17 23:59:12 UTC (rev 186977)
+++ trunk/Source/WebKit2/ChangeLog        2015-07-18 00:20:35 UTC (rev 186978)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2015-07-17  Tim Horton  &lt;timothy_horton@apple.com&gt;
+
+        [iOS] TextIndicator has a large forehead when line-height &gt; 1
+        https://bugs.webkit.org/show_bug.cgi?id=147058
+        &lt;rdar://problem/21643094&gt;
+
+        Reviewed by Dean Jackson.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::getPositionInformation):
+        Apply a review comment that I left myself and then forgot about.
+
</ins><span class="cx"> 2015-07-17  Enrica Casucci  &lt;enrica@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [iOS] Implement selectionInteractionAssistant accessor.
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPageiosWebPageIOSmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm (186977 => 186978)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm        2015-07-17 23:59:12 UTC (rev 186977)
+++ trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm        2015-07-18 00:20:35 UTC (rev 186978)
</span><span class="lines">@@ -2198,7 +2198,7 @@
</span><span class="cx">                     if (linkRange) {
</span><span class="cx">                         float deviceScaleFactor = corePage()-&gt;deviceScaleFactor();
</span><span class="cx">                         const float marginInPoints = 4;
</span><del>-                        RefPtr&lt;TextIndicator&gt; textIndicator = TextIndicator::createWithRange(*linkRange, TextIndicatorPresentationTransition::FadeIn, marginInPoints * deviceScaleFactor);
</del><ins>+                        RefPtr&lt;TextIndicator&gt; textIndicator = TextIndicator::createWithRange(*linkRange, TextIndicatorPresentationTransition::None, marginInPoints * deviceScaleFactor);
</ins><span class="cx">                         if (textIndicator)
</span><span class="cx">                             info.linkIndicator = textIndicator-&gt;data();
</span><span class="cx">                     }
</span></span></pre>
</div>
</div>

</body>
</html>