<!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>[170620] 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/170620">170620</a></dd>
<dt>Author</dt> <dd>enrica@apple.com</dd>
<dt>Date</dt> <dd>2014-06-30 17:27:25 -0700 (Mon, 30 Jun 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>REGRESSION (WK2): Weird selection behavior on autos.yahoo.com, en.wikipedia.org.
https://bugs.webkit.org/show_bug.cgi?id=134466
<rdar://problem/16817263>
Reviewed by Benjamin Poulain.
Avoid selecting blocks across frame boundaries and skip non-selectable
blocks. If the only block we find is almost the same height as the
visible area, we should not select at all.
This patch also fixes the logic to compute the next block when
shrinking the selection. When calculating the new range after shrinking,
we should not include the block that corresponds to the current handle position.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::rangeForWebSelectionAtPosition):
(WebKit::WebPage::expandedRangeFromHandle):
(WebKit::WebPage::contractedRangeFromHandle):
(WebKit::WebPage::updateSelectionWithTouches):</pre>
<h3>Modified Paths</h3>
<ul>
<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="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (170619 => 170620)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2014-07-01 00:17:09 UTC (rev 170619)
+++ trunk/Source/WebKit2/ChangeLog 2014-07-01 00:27:25 UTC (rev 170620)
</span><span class="lines">@@ -1,3 +1,24 @@
</span><ins>+2014-06-30 Enrica Casucci <enrica@apple.com>
+
+ REGRESSION (WK2): Weird selection behavior on autos.yahoo.com, en.wikipedia.org.
+ https://bugs.webkit.org/show_bug.cgi?id=134466
+ <rdar://problem/16817263>
+
+ Reviewed by Benjamin Poulain.
+
+ Avoid selecting blocks across frame boundaries and skip non-selectable
+ blocks. If the only block we find is almost the same height as the
+ visible area, we should not select at all.
+ This patch also fixes the logic to compute the next block when
+ shrinking the selection. When calculating the new range after shrinking,
+ we should not include the block that corresponds to the current handle position.
+
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::rangeForWebSelectionAtPosition):
+ (WebKit::WebPage::expandedRangeFromHandle):
+ (WebKit::WebPage::contractedRangeFromHandle):
+ (WebKit::WebPage::updateSelectionWithTouches):
+
</ins><span class="cx"> 2014-06-30 Anders Carlsson <andersca@apple.com>
</span><span class="cx">
</span><span class="cx"> WebBackForwardListItem should not store back-forward data
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPageiosWebPageIOSmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm (170619 => 170620)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm 2014-07-01 00:17:09 UTC (rev 170619)
+++ trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm 2014-07-01 00:27:25 UTC (rev 170620)
</span><span class="lines">@@ -757,11 +757,23 @@
</span><span class="cx"> return nullptr;
</span><span class="cx">
</span><span class="cx"> RenderObject* renderer = bestChoice->renderer();
</span><del>- if (renderer && renderer->childrenInline() && (renderer->isRenderBlock() && toRenderBlock(renderer)->inlineElementContinuation() == nil) && !renderer->isTable()) {
</del><ins>+ if (!renderer || renderer->style().userSelect() == SELECT_NONE)
+ return nullptr;
+
+ if (renderer->childrenInline() && (renderer->isRenderBlock() && !toRenderBlock(renderer)->inlineElementContinuation()) && !renderer->isTable()) {
</ins><span class="cx"> range = enclosingTextUnitOfGranularity(position, WordGranularity, DirectionBackward);
</span><span class="cx"> if (range && !range->collapsed(ASSERT_NO_EXCEPTION))
</span><span class="cx"> return range;
</span><span class="cx"> }
</span><ins>+
+ // If all we could find is a block whose height is very close to the height
+ // of the visible area, don't use it.
+ const float adjustmentFactor = .97;
+ boundingRectInScrollViewCoordinates = renderer->absoluteBoundingBoxRect(true);
+
+ if (boundingRectInScrollViewCoordinates.height() > m_page->mainFrame().view()->exposedContentRect().height() * adjustmentFactor)
+ return nullptr;
+
</ins><span class="cx"> flags = IsBlockSelection;
</span><span class="cx"> range = Range::create(bestChoice->document());
</span><span class="cx"> range->selectNodeContents(bestChoice, ASSERT_NO_EXCEPTION);
</span><span class="lines">@@ -1110,8 +1122,13 @@
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ distance = ceilf(distance * multiple);
+
</ins><span class="cx"> RefPtr<Range> newRange;
</span><span class="cx"> RefPtr<Range> rangeAtPosition = rangeForBlockAtPoint(testPoint);
</span><ins>+ if (&currentRange->ownerDocument() != &rangeAtPosition->ownerDocument())
+ continue;
+
</ins><span class="cx"> if (containsRange(rangeAtPosition.get(), currentRange))
</span><span class="cx"> newRange = rangeAtPosition;
</span><span class="cx"> else if (containsRange(currentRange, rangeAtPosition.get()))
</span><span class="lines">@@ -1155,8 +1172,6 @@
</span><span class="cx"> bestRange = newRange;
</span><span class="cx"> bestRect = copyRect;
</span><span class="cx"> }
</span><del>-
- distance = ceilf(distance * multiple);
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (bestRange)
</span><span class="lines">@@ -1229,13 +1244,22 @@
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ distance *= multiple;
+
</ins><span class="cx"> RefPtr<Range> newRange = rangeForBlockAtPoint(testPoint);
</span><ins>+ if (&newRange->ownerDocument() != &currentRange->ownerDocument())
+ continue;
+
</ins><span class="cx"> if (handlePosition == SelectionHandlePosition::Top || handlePosition == SelectionHandlePosition::Left)
</span><del>- newRange = Range::create(newRange->startContainer()->document(), newRange->startPosition(), currentRange->endPosition());
</del><ins>+ newRange = Range::create(newRange->startContainer()->document(), newRange->endPosition(), currentRange->endPosition());
</ins><span class="cx"> else
</span><del>- newRange = Range::create(newRange->startContainer()->document(), currentRange->startPosition(), newRange->endPosition());
</del><ins>+ newRange = Range::create(newRange->startContainer()->document(), currentRange->startPosition(), newRange->startPosition());
</ins><span class="cx">
</span><span class="cx"> IntRect copyRect = newRange->boundingBox();
</span><ins>+ if (copyRect.isEmpty()) {
+ bestRange = rangeForBlockAtPoint(testPoint);
+ break;
+ }
</ins><span class="cx"> bool isBetterChoice;
</span><span class="cx"> switch (handlePosition) {
</span><span class="cx"> case SelectionHandlePosition::Top:
</span><span class="lines">@@ -1266,24 +1290,23 @@
</span><span class="cx"> bestRect = copyRect;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- distance *= multiple;
</del><span class="cx"> }
</span><span class="cx">
</span><ins>+ if (!bestRange)
+ bestRange = currentRange;
+
</ins><span class="cx"> // If we can shrink down to text only, the only reason we wouldn't is that
</span><span class="cx"> // there are multiple sub-element blocks beneath us. If we didn't find
</span><span class="cx"> // multiple sub-element blocks, don't shrink to a sub-element block.
</span><del>- if (!bestRange) {
- bestRange = currentRange;
- Node* node = currentRange->commonAncestorContainer(ASSERT_NO_EXCEPTION);
- if (!node->isElementNode())
- node = node->parentElement();
</del><span class="cx">
</span><del>- RenderObject* renderer = node->renderer();
- if (renderer && renderer->childrenInline() && (renderer->isRenderBlock() && !toRenderBlock(renderer)->inlineElementContinuation()) && !renderer->isTable()) {
- flags = None;
- }
- }
</del><ins>+ Node* node = bestRange->commonAncestorContainer(ASSERT_NO_EXCEPTION);
+ if (!node->isElementNode())
+ node = node->parentElement();
</ins><span class="cx">
</span><ins>+ RenderObject* renderer = node->renderer();
+ if (renderer && renderer->childrenInline() && (renderer->isRenderBlock() && !toRenderBlock(renderer)->inlineElementContinuation()) && !renderer->isTable())
+ flags = None;
+
</ins><span class="cx"> return bestRange;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1430,30 +1453,30 @@
</span><span class="cx"> VisiblePosition result;
</span><span class="cx">
</span><span class="cx"> switch (static_cast<SelectionTouch>(touches)) {
</span><del>- case SelectionTouch::Started:
- case SelectionTouch::EndedNotMoving:
- break;
</del><ins>+ case SelectionTouch::Started:
+ case SelectionTouch::EndedNotMoving:
+ break;
+
+ case SelectionTouch::Ended:
+ if (frame.selection().selection().isContentEditable()) {
+ result = closestWordBoundaryForPosition(position);
+ if (result.isNotNull())
+ range = Range::create(*frame.document(), result, result);
+ } else
+ range = rangeForPosition(&frame, position, baseIsStart);
+ break;
+
+ case SelectionTouch::EndedMovingForward:
+ range = rangeAtWordBoundaryForPosition(&frame, position, baseIsStart, DirectionForward);
+ break;
</ins><span class="cx">
</span><del>- case SelectionTouch::Ended:
- if (frame.selection().selection().isContentEditable()) {
- result = closestWordBoundaryForPosition(position);
- if (result.isNotNull())
- range = Range::create(*frame.document(), result, result);
- } else
- range = rangeForPosition(&frame, position, baseIsStart);
- break;
</del><ins>+ case SelectionTouch::EndedMovingBackward:
+ range = rangeAtWordBoundaryForPosition(&frame, position, baseIsStart, DirectionBackward);
+ break;
</ins><span class="cx">
</span><del>- case SelectionTouch::EndedMovingForward:
- range = rangeAtWordBoundaryForPosition(&frame, position, baseIsStart, DirectionForward);
- break;
-
- case SelectionTouch::EndedMovingBackward:
- range = rangeAtWordBoundaryForPosition(&frame, position, baseIsStart, DirectionBackward);
- break;
-
- case SelectionTouch::Moved:
- range = rangeForPosition(&frame, position, baseIsStart);
- break;
</del><ins>+ case SelectionTouch::Moved:
+ range = rangeForPosition(&frame, position, baseIsStart);
+ break;
</ins><span class="cx"> }
</span><span class="cx"> if (range)
</span><span class="cx"> frame.selection().setSelectedRange(range.get(), position.affinity(), true);
</span></span></pre>
</div>
</div>
</body>
</html>