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

Source/core/html/HTMLTextFormControlElement.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 146 matching lines @@
     if (!placeholderElement() || placeholderValueChanged)
         updatePlaceholderText();
     HTMLElement* placeholder = placeholderElement();
     if (!placeholder)
         return;
     placeholder->setInlineStyleProperty(CSSPropertyVisibility, placeholderShouldBeVisible() ? CSSValueVisible : CSSValueHidden);
 }
 
 void HTMLTextFormControlElement::setSelectionStart(int start)
 {
-    setSelectionRange(start, max(start, selectionEnd()), selectionDirection());
+    setSelectionRange(start, max(start, selectionEndForJavascriptBindings()), selectionDirection());
 }
 
 void HTMLTextFormControlElement::setSelectionEnd(int end)
 {
-    setSelectionRange(min(end, selectionStart()), end, selectionDirection());
+    setSelectionRange(min(end, selectionStartForJavascriptBindings()), end, selectionDirection());
 }
 
 void HTMLTextFormControlElement::setSelectionDirection(const String& direction)
 {
     setSelectionRange(selectionStart(), selectionEnd(), direction);
 }
 
 void HTMLTextFormControlElement::select()
 {
     setSelectionRange(0, numeric_limits<int>::max(), SelectionHasNoDirection);
@@ skipping 95 matching lines @@
 
 void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextFieldSelectionDirection direction)
 {
     document().updateLayoutIgnorePendingStylesheets();
 
     if (!renderer() || !renderer()->isTextControl())
         return;
 
     end = max(end, 0);
     start = min(max(start, 0), end);
+    int textLength = innerTextValue().length();
+    if (textLength) {
+        if (textLength < start) {
+            start = textLength;
+            end = textLength;
+        } else if (textLength >= start && textLength < end) {

[Inactive, 2014/06/02 15:32:45]

nit: The "textLength >= start && " part seems unnecessary? (you are in the else
case of (textLength < start) so textLength has to be greater or equal to start
here).

[harpreet.sk, 2014/07/02 10:13:05]

Done.

+            end = textLength;
+        }
+    }
+    cacheSelection(start, end, direction);
+    bool isCaretSelection = (start == end);
+    bool shouldSetSelection = (document().focusedElement() == this) || (!isCaretSelection && start < textLength);

[tkent, 2014/06/01 23:40:55]

Why is !isCaretSelection necessary?

[harpreet.sk, 2014/07/02 10:13:05]

This check is necessary because when input is not focused we only shows
selection when it is not caret selection. The caret is not displayed if input is
not focused. Same behaviour  by firefox and IE.

 
-    if (!hasVisibleTextArea(renderer(), innerTextElement())) {
-        cacheSelection(start, end, direction);
+    if (!hasVisibleTextArea(renderer(), innerTextElement()))
         return;
-    }
     VisiblePosition startPosition = visiblePositionForIndex(start);
     VisiblePosition endPosition;
     if (start == end)
         endPosition = startPosition;
     else
         endPosition = visiblePositionForIndex(end);
 
     // startPosition and endPosition can be null position for example when
     // "-webkit-user-select: none" style attribute is specified.
     if (startPosition.isNotNull() && endPosition.isNotNull()) {
         ASSERT(startPosition.deepEquivalent().deprecatedNode()->shadowHost() == this
             && endPosition.deepEquivalent().deprecatedNode()->shadowHost() == this);
     }
     VisibleSelection newSelection;
     if (direction == SelectionHasBackwardDirection)
         newSelection = VisibleSelection(endPosition, startPosition);
     else
         newSelection = VisibleSelection(startPosition, endPosition);
     newSelection.setIsDirectional(direction != SelectionHasNoDirection);
 
-    if (LocalFrame* frame = document().frame())
+    LocalFrame* frame = document().frame();
+    if (frame && shouldSetSelection)
         frame->selection().setSelection(newSelection);
 }
 
 VisiblePosition HTMLTextFormControlElement::visiblePositionForIndex(int index) const
 {
     if (index <= 0)
         return VisiblePosition(firstPositionInNode(innerTextElement()), DOWNSTREAM);
     RefPtrWillBeRawPtr<Range> range = Range::create(document());
     range->selectNodeContents(innerTextElement(), ASSERT_NO_EXCEPTION);
     CharacterIterator it(range.get());
@@ skipping 10 matching lines @@
     RefPtrWillBeRawPtr<Range> range = Range::create(*indexPosition.document());
     range->setStart(innerTextElement(), 0, ASSERT_NO_EXCEPTION);
     range->setEnd(indexPosition.containerNode(), indexPosition.offsetInContainerNode(), ASSERT_NO_EXCEPTION);
     return TextIterator::rangeLength(range.get());
 }
 
 int HTMLTextFormControlElement::selectionStart() const
 {
     if (!isTextFormControl())
         return 0;
-    if (document().focusedElement() != this)
+    if (document().focusedElement() != this) {
+        if (!innerTextValue().length())

[tkent, 2014/06/01 23:40:55]

As I wrote again and again, adjusting value on selectionStart/End getters would
be wrong.
Please fix the bug without updating selectionStart() and selectionEnd, and
without adding selection*ForJavaScriptBindings().

[harpreet.sk, 2014/07/02 10:13:05]

We require a check in selectionStart() and selectionEnd() for the case when
innerEditorValue() is empty because in only that case we would always return
value 0. Consider case below:

input.selectionStart = 3;
input.selectEnd = 5;

// Right now we have not set value. So when we check selectionStart/End it
shoudlreturn 0
// Now set value

input.value = "Parsed";

// Now selectionStart/End to be 3 and 5

I had removed the api selection*ForJavaScriptBindings() and try to adjust the
value in setSelectionStart/End.

+            return 0;
         return m_cachedSelectionStart;
+    }
 
     return computeSelectionStart();
 }
 
+int HTMLTextFormControlElement::selectionStartForJavascriptBindings() const
+{
+    if (!isTextFormControl())
+        return 0;
+
+    return m_cachedSelectionStart;
+}
+
 int HTMLTextFormControlElement::computeSelectionStart() const
 {
     ASSERT(isTextFormControl());
     LocalFrame* frame = document().frame();
     if (!frame)
         return 0;
 
     return indexForVisiblePosition(VisiblePosition(frame->selection().start()));
 }
 
 int HTMLTextFormControlElement::selectionEnd() const
 {
     if (!isTextFormControl())
         return 0;
-    if (document().focusedElement() != this)
+    if (document().focusedElement() != this) {
+        if (!innerTextValue().length())

[Inactive, 2014/06/02 15:32:45]

if (innerTextValue().isEmpty())

[harpreet.sk, 2014/07/02 10:13:05]

Done.

+            return 0;
         return m_cachedSelectionEnd;
+    }
     return computeSelectionEnd();
 }
 
+int HTMLTextFormControlElement::selectionEndForJavascriptBindings() const
+{
+    if (!isTextFormControl())
+        return 0;
+
+    return m_cachedSelectionEnd;
+}
+
 int HTMLTextFormControlElement::computeSelectionEnd() const
 {
     ASSERT(isTextFormControl());
     LocalFrame* frame = document().frame();
     if (!frame)
         return 0;
 
     return indexForVisiblePosition(VisiblePosition(frame->selection().end()));
 }
 
@@ skipping 92 matching lines @@
 void HTMLTextFormControlElement::restoreCachedSelection()
 {
     setSelectionRange(m_cachedSelectionStart, m_cachedSelectionEnd, m_cachedSelectionDirection);
 }
 
 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)
+        cacheSelection(computeSelectionStart(), computeSelectionEnd(), computeSelectionDirection());
 
     if (LocalFrame* frame = document().frame()) {
         if (frame->selection().isRange() && userTriggered)
             dispatchEvent(Event::createBubble(EventTypeNames::select));
     }
 }
 
 void HTMLTextFormControlElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
     if (name == placeholderAttr) {
@@ skipping 158 matching lines @@
 
     return "ltr";
 }
 
 HTMLElement* HTMLTextFormControlElement::innerTextElement() const
 {
     return toHTMLElement(userAgentShadowRoot()->getElementById(ShadowElementNames::innerEditor()));
 }
 
 } // namespace Webcore