Commit e3141176 authored by tkent's avatar tkent Committed by Commit bot

Fix selectionDirection after setting value of TEXTAREA/INPUT.

slectionDirection wrongly returned "none" on non-Mac platforms in such
case. This CL

- makes sure that selection operations are done by setSelectionRange().
- moves direction normalization code from setSelectionRangeForBinding() to
  setSelectionRange().

Also, this makes cacheSelection() private. Only HTMLTextFormControlElement calls
it.

BUG=640861

Review-Url: https://codereview.chromium.org/2277903003
Cr-Commit-Position: refs/heads/master@{#414451}
parent 16d6f53c
...@@ -28,6 +28,7 @@ PASS selectionDirection was "forward" after hiding the element ...@@ -28,6 +28,7 @@ PASS selectionDirection was "forward" after hiding the element
PASS selectionDirection was "backward" after extending selection backward by character PASS selectionDirection was "backward" after extending selection backward by character
PASS selectionDirection was "backward" after focusing on another element PASS selectionDirection was "backward" after focusing on another element
PASS selectionDirection was "backward" after hiding the element PASS selectionDirection was "backward" after hiding the element
PASS selectionDirection was "none" after updating value
textarea on Mac textarea on Mac
PASS selectionDirection was "none" after focusing and setting selection by setSelectionRange(1, 2) PASS selectionDirection was "none" after focusing and setting selection by setSelectionRange(1, 2)
...@@ -54,6 +55,7 @@ PASS selectionDirection was "forward" after hiding the element ...@@ -54,6 +55,7 @@ PASS selectionDirection was "forward" after hiding the element
PASS selectionDirection was "backward" after extending selection backward by character PASS selectionDirection was "backward" after extending selection backward by character
PASS selectionDirection was "backward" after focusing on another element PASS selectionDirection was "backward" after focusing on another element
PASS selectionDirection was "backward" after hiding the element PASS selectionDirection was "backward" after hiding the element
PASS selectionDirection was "none" after updating value
input on Win input on Win
PASS selectionDirection was "forward" after focusing and setting selection by setSelectionRange(1, 2) PASS selectionDirection was "forward" after focusing and setting selection by setSelectionRange(1, 2)
...@@ -80,6 +82,7 @@ PASS selectionDirection was "forward" after hiding the element ...@@ -80,6 +82,7 @@ PASS selectionDirection was "forward" after hiding the element
PASS selectionDirection was "backward" after extending selection backward by character PASS selectionDirection was "backward" after extending selection backward by character
PASS selectionDirection was "backward" after focusing on another element PASS selectionDirection was "backward" after focusing on another element
PASS selectionDirection was "backward" after hiding the element PASS selectionDirection was "backward" after hiding the element
PASS selectionDirection was "forward" after updating value
textarea on Win textarea on Win
PASS selectionDirection was "forward" after focusing and setting selection by setSelectionRange(1, 2) PASS selectionDirection was "forward" after focusing and setting selection by setSelectionRange(1, 2)
...@@ -106,6 +109,7 @@ PASS selectionDirection was "forward" after hiding the element ...@@ -106,6 +109,7 @@ PASS selectionDirection was "forward" after hiding the element
PASS selectionDirection was "backward" after extending selection backward by character PASS selectionDirection was "backward" after extending selection backward by character
PASS selectionDirection was "backward" after focusing on another element PASS selectionDirection was "backward" after focusing on another element
PASS selectionDirection was "backward" after hiding the element PASS selectionDirection was "backward" after hiding the element
PASS selectionDirection was "forward" after updating value
input on Unix input on Unix
PASS selectionDirection was "forward" after focusing and setting selection by setSelectionRange(1, 2) PASS selectionDirection was "forward" after focusing and setting selection by setSelectionRange(1, 2)
...@@ -132,6 +136,7 @@ PASS selectionDirection was "forward" after hiding the element ...@@ -132,6 +136,7 @@ PASS selectionDirection was "forward" after hiding the element
PASS selectionDirection was "backward" after extending selection backward by character PASS selectionDirection was "backward" after extending selection backward by character
PASS selectionDirection was "backward" after focusing on another element PASS selectionDirection was "backward" after focusing on another element
PASS selectionDirection was "backward" after hiding the element PASS selectionDirection was "backward" after hiding the element
PASS selectionDirection was "forward" after updating value
textarea on Unix textarea on Unix
PASS selectionDirection was "forward" after focusing and setting selection by setSelectionRange(1, 2) PASS selectionDirection was "forward" after focusing and setting selection by setSelectionRange(1, 2)
...@@ -158,6 +163,7 @@ PASS selectionDirection was "forward" after hiding the element ...@@ -158,6 +163,7 @@ PASS selectionDirection was "forward" after hiding the element
PASS selectionDirection was "backward" after extending selection backward by character PASS selectionDirection was "backward" after extending selection backward by character
PASS selectionDirection was "backward" after focusing on another element PASS selectionDirection was "backward" after focusing on another element
PASS selectionDirection was "backward" after hiding the element PASS selectionDirection was "backward" after hiding the element
PASS selectionDirection was "forward" after updating value
PASS successfullyParsed is true PASS successfullyParsed is true
......
...@@ -77,6 +77,9 @@ function runTest(element, platform) { ...@@ -77,6 +77,9 @@ function runTest(element, platform) {
assertDirection('backward', element, 'extending selection backward by character'); assertDirection('backward', element, 'extending selection backward by character');
testCache('backward', element); testCache('backward', element);
element.blur();
element.value = element.value + 'foo';
assertDirection(noneOnMacAndForwardOnOthers, element, 'updating value');
} }
function runTestFor(platform) { function runTestFor(platform) {
......
...@@ -991,11 +991,7 @@ void HTMLInputElement::setEditingValue(const String& value) ...@@ -991,11 +991,7 @@ void HTMLInputElement::setEditingValue(const String& value)
subtreeHasChanged(); subtreeHasChanged();
unsigned max = value.length(); unsigned max = value.length();
if (focused()) setSelectionRange(max, max, SelectionHasNoDirection, NotDispatchSelectEvent);
setSelectionRange(max, max, SelectionHasNoDirection, NotDispatchSelectEvent);
else
cacheSelectionInResponseToSetValue(max);
dispatchInputEvent(); dispatchInputEvent();
} }
......
...@@ -221,8 +221,6 @@ public: ...@@ -221,8 +221,6 @@ public:
bool needsToUpdateViewValue() const { return m_needsToUpdateViewValue; } bool needsToUpdateViewValue() const { return m_needsToUpdateViewValue; }
void setInnerEditorValue(const String&) override; void setInnerEditorValue(const String&) override;
void cacheSelectionInResponseToSetValue(int caretOffset) { cacheSelection(caretOffset, caretOffset, SelectionHasNoDirection); }
// For test purposes. // For test purposes.
void selectColorInColorChooser(const Color&); void selectColorInColorChooser(const Color&);
void endColorChooser(); void endColorChooser();
......
...@@ -270,11 +270,7 @@ void HTMLTextFormControlElement::setSelectionRangeForBinding(int start, int end, ...@@ -270,11 +270,7 @@ void HTMLTextFormControlElement::setSelectionRangeForBinding(int start, int end,
direction = SelectionHasForwardDirection; direction = SelectionHasForwardDirection;
else if (directionString == "backward") else if (directionString == "backward")
direction = SelectionHasBackwardDirection; direction = SelectionHasBackwardDirection;
setSelectionRange(start, end, direction);
if (direction == SelectionHasNoDirection && document().frame() && document().frame()->editor().behavior().shouldConsiderSelectionAsDirectional())
direction = SelectionHasForwardDirection;
return setSelectionRange(start, end, direction);
} }
static Position positionForIndex(HTMLElement* innerEditor, int index) static Position positionForIndex(HTMLElement* innerEditor, int index)
...@@ -351,6 +347,9 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextField ...@@ -351,6 +347,9 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextField
DCHECK_GE(editorValueLength, 0); DCHECK_GE(editorValueLength, 0);
end = std::max(std::min(end, editorValueLength), 0); end = std::max(std::min(end, editorValueLength), 0);
start = std::min(std::max(start, 0), end); start = std::min(std::max(start, 0), end);
LocalFrame* frame = document().frame();
if (direction == SelectionHasNoDirection && frame && frame->editor().behavior().shouldConsiderSelectionAsDirectional())
direction = SelectionHasForwardDirection;
cacheSelection(start, end, direction); cacheSelection(start, end, direction);
if (document().focusedElement() != this) { if (document().focusedElement() != this) {
...@@ -359,7 +358,6 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextField ...@@ -359,7 +358,6 @@ void HTMLTextFormControlElement::setSelectionRange(int start, int end, TextField
return; return;
} }
LocalFrame* frame = document().frame();
HTMLElement* innerEditor = innerEditorElement(); HTMLElement* innerEditor = innerEditorElement();
if (!frame || !innerEditor) if (!frame || !innerEditor)
return; return;
...@@ -471,11 +469,10 @@ static const AtomicString& directionString(TextFieldSelectionDirection direction ...@@ -471,11 +469,10 @@ static const AtomicString& directionString(TextFieldSelectionDirection direction
const AtomicString& HTMLTextFormControlElement::selectionDirection() const const AtomicString& HTMLTextFormControlElement::selectionDirection() const
{ {
if (!isTextFormControl()) // Ensured by HTMLInputElement::selectionDirectionForBinding().
return directionString(SelectionHasNoDirection); DCHECK(isTextFormControl());
if (document().focusedElement() != this) if (document().focusedElement() != this)
return directionString(m_cachedSelectionDirection); return directionString(m_cachedSelectionDirection);
return directionString(computeSelectionDirection()); return directionString(computeSelectionDirection());
} }
......
...@@ -70,11 +70,11 @@ public: ...@@ -70,11 +70,11 @@ public:
void select(); void select();
virtual void setRangeText(const String& replacement, ExceptionState&); virtual void setRangeText(const String& replacement, ExceptionState&);
virtual void setRangeText(const String& replacement, unsigned start, unsigned end, const String& selectionMode, ExceptionState&); virtual void setRangeText(const String& replacement, unsigned start, unsigned end, const String& selectionMode, ExceptionState&);
// Web-exposed setSelectionRange() function. This translates "none" // Web-exposed setSelectionRange() function. This schedule to dispatch
// direction to "forward" if necessary. // 'select' event.
void setSelectionRangeForBinding(int start, int end, const String& direction = "none"); void setSelectionRangeForBinding(int start, int end, const String& direction = "none");
// Blink-internal version of setSelectionRange(). This never translates // Blink-internal version of setSelectionRange(). This translates "none"
// "none" direction. // direction to "forward" on platforms without "none" direction.
void setSelectionRange(int start, int end, TextFieldSelectionDirection = SelectionHasNoDirection, NeedToDispatchSelectEvent = DispatchSelectEvent); void setSelectionRange(int start, int end, TextFieldSelectionDirection = SelectionHasNoDirection, NeedToDispatchSelectEvent = DispatchSelectEvent);
Range* selection() const; Range* selection() const;
...@@ -115,14 +115,6 @@ protected: ...@@ -115,14 +115,6 @@ protected:
void parseAttribute(const QualifiedName&, const AtomicString&, const AtomicString&) override; void parseAttribute(const QualifiedName&, const AtomicString&, const AtomicString&) override;
void cacheSelection(int start, int end, TextFieldSelectionDirection direction)
{
DCHECK_GE(start, 0);
m_cachedSelectionStart = start;
m_cachedSelectionEnd = end;
m_cachedSelectionDirection = direction;
}
void restoreCachedSelection(); void restoreCachedSelection();
void defaultEventHandler(Event*) override; void defaultEventHandler(Event*) override;
...@@ -139,6 +131,15 @@ private: ...@@ -139,6 +131,15 @@ private:
int computeSelectionStart() const; int computeSelectionStart() const;
int computeSelectionEnd() const; int computeSelectionEnd() const;
TextFieldSelectionDirection computeSelectionDirection() const; TextFieldSelectionDirection computeSelectionDirection() const;
void cacheSelection(int start, int end, TextFieldSelectionDirection direction)
{
DCHECK_GE(start, 0);
// TODO(tkent): Add DCHECK_LE(start, end). It breaks
// editing/selection/select-across-readonly-input-{1,4}.html
m_cachedSelectionStart = start;
m_cachedSelectionEnd = end;
m_cachedSelectionDirection = direction;
}
void dispatchFocusEvent(Element* oldFocusedElement, WebFocusType, InputDeviceCapabilities* sourceCapabilities) final; void dispatchFocusEvent(Element* oldFocusedElement, WebFocusType, InputDeviceCapabilities* sourceCapabilities) final;
void dispatchBlurEvent(Element* newFocusedElement, WebFocusType, InputDeviceCapabilities* sourceCapabilities) final; void dispatchBlurEvent(Element* newFocusedElement, WebFocusType, InputDeviceCapabilities* sourceCapabilities) final;
......
...@@ -163,10 +163,7 @@ void TextFieldInputType::setValue(const String& sanitizedValue, bool valueChange ...@@ -163,10 +163,7 @@ void TextFieldInputType::setValue(const String& sanitizedValue, bool valueChange
element().updateView(); element().updateView();
unsigned max = visibleValue().length(); unsigned max = visibleValue().length();
if (element().focused()) element().setSelectionRange(max, max, SelectionHasNoDirection, NotDispatchSelectEvent);
element().setSelectionRange(max, max, SelectionHasNoDirection, NotDispatchSelectEvent);
else
element().cacheSelectionInResponseToSetValue(max);
if (!valueChanged) if (!valueChanged)
return; return;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment