setSelectionRange shouldn't trigger a synchronous layout to check focusability when...

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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@164156 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index ccf5e75dd5ff6667d4a8b18f38951abdeb74e9a8..0e4b0abe3c7541e09ae579924896a296cca16845 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,23 @@
+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.
+
 2014-02-14  Dan Bernstein  <mitz@apple.com>
 
         REGRESSION (r157443): Search fields with a non-white background don’t have a round bezel

diff --git a/Source/WebCore/editing/FrameSelection.cpp b/Source/WebCore/editing/FrameSelection.cpp
index 50331cf41678de9d6b1b05f90b7be483cfa3452b..362bfb4d34062fe404b60d173399cc49f4a7cae5 100644 (file)
--- a/Source/WebCore/editing/FrameSelection.cpp
+++ b/Source/WebCore/editing/FrameSelection.cpp
@@ -1640,7 +1640,8 @@ void FrameSelection::selectAll()
 {
     Document* document = m_frame->document();
 
-    if (document->focusedElement() && document->focusedElement()->hasTagName(selectTag)) {
+    Element* focusedElement = document->focusedElement();
+    if (focusedElement && focusedElement->hasTagName(selectTag)) {
         HTMLSelectElement* selectElement = toHTMLSelectElement(document->focusedElement());
         if (selectElement->canSelectAll()) {
             selectElement->selectAll();
@@ -1657,7 +1658,15 @@ void FrameSelection::selectAll()
         else
             selectStartTarget = root.get();
     } else {
-        root = m_selection.nonBoundaryShadowTreeRootNode();
+        if (m_selection.isNone() && focusedElement) {
+            if (focusedElement->isTextFormControl()) {
+                toHTMLTextFormControlElement(focusedElement)->select();
+                return;
+            }
+            root = focusedElement->nonBoundaryShadowTreeRootNode();
+        } else
+            root = m_selection.nonBoundaryShadowTreeRootNode();
+
         if (root)
             selectStartTarget = root->shadowHost();
         else {

diff --git a/Source/WebCore/html/HTMLTextFormControlElement.cpp b/Source/WebCore/html/HTMLTextFormControlElement.cpp
index 3eae47ae8068eb1adc56134b0a19398407d9eef7..9fb51d660bbbe5026fe15cf9c79a66d47514362d 100644 (file)
--- a/Source/WebCore/html/HTMLTextFormControlElement.cpp
+++ b/Source/WebCore/html/HTMLTextFormControlElement.cpp
@@ -213,11 +213,6 @@ void HTMLTextFormControlElement::dispatchFormControlChangeEvent()
     setChangedSinceLastFormControlChangeEvent(false);
 }
 
-static inline bool hasVisibleTextArea(RenderElement& textControl, TextControlInnerTextElement* innerText)
-{
-    return textControl.style().visibility() != HIDDEN && innerText && innerText->renderer() && innerText->renderBox()->height();
-}
-
 void HTMLTextFormControlElement::setRangeText(const String& replacement, ExceptionCode& ec)
 {
     setRangeText(replacement, selectionStart(), selectionEnd(), String(), ec);
@@ -290,19 +285,25 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, const Str
 
 void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextFieldSelectionDirection direction)
 {
-    document().updateLayoutIgnorePendingStylesheets();
-
-    if (!renderer() || !renderer()->isTextControl())
+    if (!isTextFormControl())
         return;
 
     end = std::max(end, 0);
     start = std::min(std::max(start, 0), end);
 
     TextControlInnerTextElement* innerText = innerTextElement();
-    if (!hasVisibleTextArea(*renderer(), innerText)) {
-        cacheSelection(start, end, direction);
-        return;
+    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;
+            }
+        }
     }
+
     Position startPosition = positionForIndex(innerText, start);
     Position endPosition;
     if (start == end)
@@ -317,8 +318,11 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextField
         newSelection = VisibleSelection(startPosition, endPosition);
     newSelection.setIsDirectional(direction != SelectionHasNoDirection);
 
+    FrameSelection::SetSelectionOptions options = FrameSelection::defaultSetSelectionOptions();
+    if (hasFocus)
+        options |= FrameSelection::DoNotSetFocus;
     if (Frame* frame = document().frame())
-        frame->selection().setSelection(newSelection);
+        frame->selection().setSelection(newSelection, options);
 }
 
 int HTMLTextFormControlElement::indexForVisiblePosition(const VisiblePosition& position) const