<!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>[164156] trunk/Source/WebCore</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/164156">164156</a></dd>
<dt>Author</dt> <dd>rniwa@webkit.org</dd>
<dt>Date</dt> <dd>2014-02-14 20:52:38 -0800 (Fri, 14 Feb 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>setSelectionRange shouldn't trigger a synchronous layout to check focusability when text field is already focused
https://bugs.webkit.org/show_bug.cgi?id=128804
Reviewed by Enrica Casucci.
Don't trigger a synchronous layout at the beginning of setSelectionRange if the element is already focused
since we don't have to check the size of render box in that case.
We should be able to get rid of this synchronous layout entirely once we fix https://webkit.org/b/128797
but that's somewhat risky behavioral change so we'll do that in a separate patch.
* editing/FrameSelection.cpp:
(WebCore::FrameSelection::selectAll): Fixed the bug where selectAll selects the entire document even if the text
form contol is focused if the selection is none (i.e. not anchored to any node).
* html/HTMLTextFormControlElement.cpp:
(WebCore::HTMLTextFormControlElement::setSelectionRange): Only update the layout if the element is not focused
already. Also pass in DoNotSetFocus option to setSelection since we already have the focus in that case.</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="#trunkSourceWebCorehtmlHTMLTextFormControlElementcpp">trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (164155 => 164156)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-02-15 04:33:20 UTC (rev 164155)
+++ trunk/Source/WebCore/ChangeLog 2014-02-15 04:52:38 UTC (rev 164156)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2014-02-14 Ryosuke Niwa <rniwa@webkit.org>
+
+ setSelectionRange shouldn't trigger a synchronous layout to check focusability when text field is already focused
+ https://bugs.webkit.org/show_bug.cgi?id=128804
+
+ Reviewed by Enrica Casucci.
+
+ Don't trigger a synchronous layout at the beginning of setSelectionRange if the element is already focused
+ since we don't have to check the size of render box in that case.
+
+ We should be able to get rid of this synchronous layout entirely once we fix https://webkit.org/b/128797
+ but that's somewhat risky behavioral change so we'll do that in a separate patch.
+
+ * editing/FrameSelection.cpp:
+ (WebCore::FrameSelection::selectAll): Fixed the bug where selectAll selects the entire document even if the text
+ form contol is focused if the selection is none (i.e. not anchored to any node).
+ * html/HTMLTextFormControlElement.cpp:
+ (WebCore::HTMLTextFormControlElement::setSelectionRange): Only update the layout if the element is not focused
+ already. Also pass in DoNotSetFocus option to setSelection since we already have the focus in that case.
+
</ins><span class="cx"> 2014-02-14 Dan Bernstein <mitz@apple.com>
</span><span class="cx">
</span><span class="cx"> REGRESSION (r157443): Search fields with a non-white background don’t have a round bezel
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingFrameSelectioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (164155 => 164156)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/editing/FrameSelection.cpp 2014-02-15 04:33:20 UTC (rev 164155)
+++ trunk/Source/WebCore/editing/FrameSelection.cpp 2014-02-15 04:52:38 UTC (rev 164156)
</span><span class="lines">@@ -1640,7 +1640,8 @@
</span><span class="cx"> {
</span><span class="cx"> Document* document = m_frame->document();
</span><span class="cx">
</span><del>- if (document->focusedElement() && document->focusedElement()->hasTagName(selectTag)) {
</del><ins>+ Element* focusedElement = document->focusedElement();
+ if (focusedElement && focusedElement->hasTagName(selectTag)) {
</ins><span class="cx"> HTMLSelectElement* selectElement = toHTMLSelectElement(document->focusedElement());
</span><span class="cx"> if (selectElement->canSelectAll()) {
</span><span class="cx"> selectElement->selectAll();
</span><span class="lines">@@ -1657,7 +1658,15 @@
</span><span class="cx"> else
</span><span class="cx"> selectStartTarget = root.get();
</span><span class="cx"> } else {
</span><del>- root = m_selection.nonBoundaryShadowTreeRootNode();
</del><ins>+ if (m_selection.isNone() && focusedElement) {
+ if (focusedElement->isTextFormControl()) {
+ toHTMLTextFormControlElement(focusedElement)->select();
+ return;
+ }
+ root = focusedElement->nonBoundaryShadowTreeRootNode();
+ } else
+ root = m_selection.nonBoundaryShadowTreeRootNode();
+
</ins><span class="cx"> if (root)
</span><span class="cx"> selectStartTarget = root->shadowHost();
</span><span class="cx"> else {
</span></span></pre></div>
<a id="trunkSourceWebCorehtmlHTMLTextFormControlElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp (164155 => 164156)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp 2014-02-15 04:33:20 UTC (rev 164155)
+++ trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp 2014-02-15 04:52:38 UTC (rev 164156)
</span><span class="lines">@@ -213,11 +213,6 @@
</span><span class="cx"> setChangedSinceLastFormControlChangeEvent(false);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static inline bool hasVisibleTextArea(RenderElement& textControl, TextControlInnerTextElement* innerText)
-{
- return textControl.style().visibility() != HIDDEN && innerText && innerText->renderer() && innerText->renderBox()->height();
-}
-
</del><span class="cx"> void HTMLTextFormControlElement::setRangeText(const String& replacement, ExceptionCode& ec)
</span><span class="cx"> {
</span><span class="cx"> setRangeText(replacement, selectionStart(), selectionEnd(), String(), ec);
</span><span class="lines">@@ -290,19 +285,25 @@
</span><span class="cx">
</span><span class="cx"> void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextFieldSelectionDirection direction)
</span><span class="cx"> {
</span><del>- document().updateLayoutIgnorePendingStylesheets();
-
- if (!renderer() || !renderer()->isTextControl())
</del><ins>+ if (!isTextFormControl())
</ins><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> end = std::max(end, 0);
</span><span class="cx"> start = std::min(std::max(start, 0), end);
</span><span class="cx">
</span><span class="cx"> TextControlInnerTextElement* innerText = innerTextElement();
</span><del>- if (!hasVisibleTextArea(*renderer(), innerText)) {
- cacheSelection(start, end, direction);
- return;
</del><ins>+ bool hasFocus = document().focusedElement() == this;
+ if (!hasFocus && innerText) {
+ // FIXME: Removing this synchronous layout requires fixing <https://webkit.org/b/128797>
+ document().updateLayoutIgnorePendingStylesheets();
+ if (RenderElement* rendererTextControl = renderer()) {
+ if (rendererTextControl->style().visibility() == HIDDEN || !innerText->renderBox()->height()) {
+ cacheSelection(start, end, direction);
+ return;
+ }
+ }
</ins><span class="cx"> }
</span><ins>+
</ins><span class="cx"> Position startPosition = positionForIndex(innerText, start);
</span><span class="cx"> Position endPosition;
</span><span class="cx"> if (start == end)
</span><span class="lines">@@ -317,8 +318,11 @@
</span><span class="cx"> newSelection = VisibleSelection(startPosition, endPosition);
</span><span class="cx"> newSelection.setIsDirectional(direction != SelectionHasNoDirection);
</span><span class="cx">
</span><ins>+ FrameSelection::SetSelectionOptions options = FrameSelection::defaultSetSelectionOptions();
+ if (hasFocus)
+ options |= FrameSelection::DoNotSetFocus;
</ins><span class="cx"> if (Frame* frame = document().frame())
</span><del>- frame->selection().setSelection(newSelection);
</del><ins>+ frame->selection().setSelection(newSelection, options);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> int HTMLTextFormControlElement::indexForVisiblePosition(const VisiblePosition& position) const
</span></span></pre>
</div>
</div>
</body>
</html>