Blink does not respect input.selectionStart and input.selectionEnd for some cases

Source/core/html/HTMLTextFormControlElement.cpp

Index: Source/core/html/HTMLTextFormControlElement.cpp
diff --git a/Source/core/html/HTMLTextFormControlElement.cpp b/Source/core/html/HTMLTextFormControlElement.cpp
index 0434728f24a557dcb6aa4a94af75ce08ceaccc60..eb3ef54c466a7b70b6383b21cd449046dd8631ab 100644
--- a/Source/core/html/HTMLTextFormControlElement.cpp
+++ b/Source/core/html/HTMLTextFormControlElement.cpp
@@ -163,12 +163,18 @@ void HTMLTextFormControlElement::updatePlaceholderVisibility(bool placeholderVal
 
 void HTMLTextFormControlElement::setSelectionStart(int start)
 {
-    setSelectionRange(start, std::max(start, selectionEnd()), selectionDirection());
+    if (!innerEditorValue().isEmpty())

[yosin_UTC9, 2014/07/03 01:58:36]

It is better to call setSelectionRange(start, selectionEnd(),
selectionDirection()) then adjust start and end in setSelectionRange.

[harpreet.sk, 2014/07/04 13:50:26]

Adjusting values in setSelectionRange

+        setSelectionRange(start, std::max(start, selectionEnd()), selectionDirection());
+    else
+        setSelectionRange(start, std::max(start, m_cachedSelectionEnd), selectionDirection());

[yosin_UTC9, 2014/07/03 01:58:35]

Since, control has empty string, we can set 0, 0 to selection.

[harpreet.sk, 2014/07/04 13:50:27]

This condition is removed from here and adjusting values in setSelectionRange

 }
 
 void HTMLTextFormControlElement::setSelectionEnd(int end)
 {
-    setSelectionRange(std::min(end, selectionStart()), end, selectionDirection());
+    if (!innerEditorValue().isEmpty())
+        setSelectionRange(std::min(end, selectionStart()), end, selectionDirection());
+    else
+        setSelectionRange(std::min(end, m_cachedSelectionStart), end, selectionDirection());

[yosin_UTC9, 2014/07/03 01:58:36]

Since, control has empty string, we can set 0, 0 to selection.

[harpreet.sk, 2014/07/04 13:50:26]

ditto

 }
 
 void HTMLTextFormControlElement::setSelectionDirection(const String& direction)
@@ -284,11 +290,21 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextField
 
     end = std::max(end, 0);

[yosin_UTC9, 2014/07/03 01:58:35]

How about
int textLength = innerEditorValue.length();
end = std::max(std::min(end, length), 0);
start = std::max(std::min(start, end), 0);

[harpreet.sk, 2014/07/04 13:50:27]

thanks for the suggestion... incorporated in new patch.

     start = std::min(std::max(start, 0), end);
+    int textLength = innerEditorValue().length();
+    if (textLength) {
+        if (textLength < start) {
+            start = textLength;
+            end = textLength;
+        } else if (textLength < end) {
+            end = textLength;
+        }
+    }
+    cacheSelection(start, end, direction);
+    bool isCaretSelection = (start == end);

[yosin_UTC9, 2014/07/03 01:58:35]

nit: we don't need to have surrounding parenthesis.

[harpreet.sk, 2014/07/04 13:50:27]

Done.

+    bool shouldSetSelection = (document().focusedElement() == this) || (!isCaretSelection && start < textLength);
 
-    if (!hasVisibleTextArea(renderer(), innerEditorElement())) {
-        cacheSelection(start, end, direction);
+    if (!hasVisibleTextArea(renderer(), innerEditorElement()))

[yosin_UTC9, 2014/07/03 01:58:35]

It seems we should set |FrameSelection| if text area has focus. But this isn't
related to this patch.

[harpreet.sk, 2014/07/04 13:50:26]

this check was already there ... i just moved the calling of cacheSelection from
the inside of the check.. this check is not part of this bug

         return;
-    }
     VisiblePosition startPosition = visiblePositionForIndex(start);
     VisiblePosition endPosition;
     if (start == end)
@@ -309,7 +325,8 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextField
         newSelection = VisibleSelection(startPosition, endPosition);
     newSelection.setIsDirectional(direction != SelectionHasNoDirection);
 
-    if (LocalFrame* frame = document().frame())
+    LocalFrame* frame = document().frame();
+    if (frame && shouldSetSelection)

[yosin_UTC9, 2014/07/03 01:58:35]

Please move this if-statement just after L306 if (!hasVisibleTextArea(...).
When we don't have |LocalFrame| and |!shouldSelection| we don't need to
construct |VisibleSelection|.

[harpreet.sk, 2014/07/04 13:50:27]

Done.

         frame->selection().setSelection(newSelection);
 }
 
@@ -338,11 +355,10 @@ int HTMLTextFormControlElement::indexForVisiblePosition(const VisiblePosition& p
 
 int HTMLTextFormControlElement::selectionStart() const
 {
-    if (!isTextFormControl())
+    if (!isTextFormControl() || innerEditorValue().isEmpty())

[yosin_UTC9, 2014/07/03 01:58:35]

We should reset |m_cachedSelectionStart| when value is changed. Rather than
check whether value is empty or not.

[harpreet.sk, 2014/07/04 13:50:27]

Done.

         return 0;
     if (document().focusedElement() != this)
         return m_cachedSelectionStart;
-
     return computeSelectionStart();
 }
 
@@ -358,7 +374,7 @@ int HTMLTextFormControlElement::computeSelectionStart() const
 
 int HTMLTextFormControlElement::selectionEnd() const
 {
-    if (!isTextFormControl())
+    if (!isTextFormControl() || innerEditorValue().isEmpty())

[yosin_UTC9, 2014/07/03 01:58:35]

We should reset |m_cachedSelectionEnd| when value is changed. Rather than check
whether value is empty or not.

[harpreet.sk, 2014/07/04 13:50:27]

Done.

         return 0;
     if (document().focusedElement() != this)
         return m_cachedSelectionEnd;
@@ -477,8 +493,8 @@ void HTMLTextFormControlElement::selectionChanged(bool userTriggered)
     if (!renderer() || !isTextFormControl())
         return;
 
-    // selectionStart() or selectionEnd() will return cached selection when this node doesn't have focus
-    cacheSelection(computeSelectionStart(), computeSelectionEnd(), computeSelectionDirection());
+    if (document().focusedElement() == this)

[yosin_UTC9, 2014/07/03 01:58:36]

We should keep cached selection up-to date.

[harpreet.sk, 2014/07/04 13:50:26]

The cached selection is getting up-to-date in setSelectionRange.

+        cacheSelection(computeSelectionStart(), computeSelectionEnd(), computeSelectionDirection());
 
     if (LocalFrame* frame = document().frame()) {
         if (frame->selection().isRange() && userTriggered)