Skip to content

Commit

Permalink
Bug 1343037 part 6. Simplify the setup around the editor state's GetS…
Browse files Browse the repository at this point in the history 
…electionRange function. r=ehsan

Really, there are only two cases we need to worry about.  Either
IsSelectionCached(), and then our SelectionProperties has the data we want, or
not and then we have a non-null mSelCon which has the data we want.

Since we are now using cached selection state a lot more (instead of
initializing the editor whenever someone asks for selection state), we need to
actually update it more correctly when .value is set.

And since we now update the cached selection state for the case when .value has
been set (to point to the end of the text), we need to change
HTMLInputElement::HasCachedSelection to return false for that case.  Otherwise
we will always do eager editor init on value set.  We handle that by not doing
eager init if the cached selection is collapsed.

The web platform test changes test the "update on .value set" behavior.  They
fail without this patch, pass with it.

MozReview-Commit-ID: DDU8U4MGb23
  • Loading branch information
@bzbarsky
bzbarsky committed Mar 9, 2017
1 parent 56f5644 commit 77e7a55
Show file tree
Hide file tree
Showing 10 changed files with 229 additions and 210 deletions.
1 change: 0 additions & 1 deletion accessible/tests/mochitest/jsat/test_content_text.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@
atStart: true,
atEnd: true
}, { focused: 'input[type=text]' }),
new ExpectedTextSelectionChanged(0, 0),
new ExpectedTextSelectionChanged(0, 0)
],
[ContentMessages.simpleMovePrevious,
Expand Down
78 changes: 25 additions & 53 deletions dom/html/HTMLInputElement.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2010,7 +2010,8 @@ HTMLInputElement::SetValue(const nsAString& aValue, CallerType aCallerType,

nsresult rv =
SetValueInternal(aValue, nsTextEditorState::eSetValue_ByContent |
nsTextEditorState::eSetValue_Notify);
nsTextEditorState::eSetValue_Notify |
nsTextEditorState::eSetValue_MoveCursorToEnd);
if (NS_FAILED(rv)) {
aRv.Throw(rv);
return;
Expand All @@ -2022,7 +2023,8 @@ HTMLInputElement::SetValue(const nsAString& aValue, CallerType aCallerType,
} else {
nsresult rv =
SetValueInternal(aValue, nsTextEditorState::eSetValue_ByContent |
nsTextEditorState::eSetValue_Notify);
nsTextEditorState::eSetValue_Notify |
nsTextEditorState::eSetValue_MoveCursorToEnd);
if (NS_FAILED(rv)) {
aRv.Throw(rv);
return;
Expand Down Expand Up @@ -2866,8 +2868,10 @@ HTMLInputElement::SetUserInput(const nsAString& aValue)
return rv.StealNSResult();
} else {
nsresult rv =
SetValueInternal(aValue, nsTextEditorState::eSetValue_BySetUserInput |
nsTextEditorState::eSetValue_Notify);
SetValueInternal(aValue,
nsTextEditorState::eSetValue_BySetUserInput |
nsTextEditorState::eSetValue_Notify|
nsTextEditorState::eSetValue_MoveCursorToEnd);
NS_ENSURE_SUCCESS(rv, rv);
}

Expand Down Expand Up @@ -6321,14 +6325,9 @@ HTMLInputElement::SetRangeText(const nsAString& aReplacement, ErrorResult& aRv)
}

int32_t start, end;
aRv = GetSelectionRange(&start, &end);
GetSelectionRange(&start, &end, aRv);
if (aRv.Failed()) {
nsTextEditorState* state = GetEditorState();
if (state && state->IsSelectionCached()) {
start = state->GetSelectionProperties().GetStart();
end = state->GetSelectionProperties().GetEnd();
aRv = NS_OK;
}
return;
}

SetRangeText(aReplacement, start, end, mozilla::dom::SelectionMode::Preserve,
Expand Down Expand Up @@ -6364,14 +6363,9 @@ HTMLInputElement::SetRangeText(const nsAString& aReplacement, uint32_t aStart,
}

if (aSelectionStart == -1 && aSelectionEnd == -1) {
aRv = GetSelectionRange(&aSelectionStart, &aSelectionEnd);
GetSelectionRange(&aSelectionStart, &aSelectionEnd, aRv);
if (aRv.Failed()) {
nsTextEditorState* state = GetEditorState();
if (state && state->IsSelectionCached()) {
aSelectionStart = state->GetSelectionProperties().GetStart();
aSelectionEnd = state->GetSelectionProperties().GetEnd();
aRv = NS_OK;
}
return;
}
}

Expand Down Expand Up @@ -6436,17 +6430,7 @@ HTMLInputElement::GetSelectionStart(ErrorResult& aRv)
}

int32_t selEnd, selStart;
nsresult rv = GetSelectionRange(&selStart, &selEnd);

if (NS_FAILED(rv)) {
nsTextEditorState* state = GetEditorState();
if (state && state->IsSelectionCached()) {
return Nullable<int32_t>(state->GetSelectionProperties().GetStart());
}
aRv.Throw(rv);
return Nullable<int32_t>();
}

GetSelectionRange(&selStart, &selEnd, aRv);
return Nullable<int32_t>(selStart);
}

Expand Down Expand Up @@ -6477,7 +6461,7 @@ HTMLInputElement::SetSelectionStart(const Nullable<int32_t>& aSelectionStart,
}

int32_t start, end;
aRv = GetSelectionRange(&start, &end);
GetSelectionRange(&start, &end, aRv);
if (aRv.Failed()) {
return;
}
Expand All @@ -6498,17 +6482,7 @@ HTMLInputElement::GetSelectionEnd(ErrorResult& aRv)
}

int32_t selEnd, selStart;
nsresult rv = GetSelectionRange(&selStart, &selEnd);

if (NS_FAILED(rv)) {
nsTextEditorState* state = GetEditorState();
if (state && state->IsSelectionCached()) {
return Nullable<int32_t>(state->GetSelectionProperties().GetEnd());
}
aRv.Throw(rv);
return Nullable<int32_t>();
}

GetSelectionRange(&selStart, &selEnd, aRv);
return Nullable<int32_t>(selEnd);
}

Expand Down Expand Up @@ -6539,7 +6513,7 @@ HTMLInputElement::SetSelectionEnd(const Nullable<int32_t>& aSelectionEnd,
}

int32_t start, end;
aRv = GetSelectionRange(&start, &end);
GetSelectionRange(&start, &end, aRv);
if (aRv.Failed()) {
return;
}
Expand All @@ -6560,22 +6534,19 @@ HTMLInputElement::GetFiles(nsIDOMFileList** aFileList)
return NS_OK;
}

NS_IMETHODIMP
void
HTMLInputElement::GetSelectionRange(int32_t* aSelectionStart,
int32_t* aSelectionEnd)
int32_t* aSelectionEnd,
ErrorResult& aRv)
{
// Flush frames, because our editor state will want to work with the frame.
if (IsInComposedDoc()) {
GetComposedDoc()->FlushPendingNotifications(FlushType::Frames);
}

nsTextEditorState* state = GetEditorState();
if (!state) {
// Not a text control.
return NS_ERROR_FAILURE;
aRv.Throw(NS_ERROR_UNEXPECTED);
return;
}

return state->GetSelectionRange(aSelectionStart, aSelectionEnd);
state->GetSelectionRange(aSelectionStart, aSelectionEnd, aRv);
}

static void
Expand Down Expand Up @@ -6654,7 +6625,7 @@ HTMLInputElement::SetSelectionDirection(const nsAString& aDirection, ErrorResult
}

int32_t start, end;
aRv = GetSelectionRange(&start, &end);
GetSelectionRange(&start, &end, aRv);
if (!aRv.Failed()) {
aRv = SetSelectionRange(start, end, aDirection);
}
Expand Down Expand Up @@ -8518,7 +8489,8 @@ HTMLInputElement::HasCachedSelection()
if (state) {
isCached = state->IsSelectionCached() &&
state->HasNeverInitializedBefore() &&
!state->GetSelectionProperties().IsDefault();
state->GetSelectionProperties().GetStart() !=
state->GetSelectionProperties().GetEnd();
if (isCached) {
state->WillInitEagerly();
}
Expand Down
8 changes: 4 additions & 4 deletions dom/html/HTMLInputElement.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,9 @@ class HTMLInputElement final : public nsGenericHTMLFormElementWithState,
NS_IMETHOD_(void) InitializeKeyboardEventListeners() override;
NS_IMETHOD_(void) OnValueChanged(bool aNotify, bool aWasInteractiveUserChange) override;
NS_IMETHOD_(bool) HasCachedSelection() override;
NS_IMETHOD GetSelectionRange(int32_t* aSelectionStart,
int32_t* aSelectionEnd) override;
virtual void GetSelectionRange(int32_t* aSelectionStart,
int32_t* aSelectionEnd,
ErrorResult& aRv) override;

void GetDisplayFileName(nsAString& aFileName) const;

Expand Down Expand Up @@ -293,11 +294,10 @@ class HTMLInputElement final : public nsGenericHTMLFormElementWithState,

void MaybeLoadImage();

void SetSelectionProperties(const nsTextEditorState::SelectionProperties& aProps)
void SetSelectionCached()
{
MOZ_ASSERT(mType == NS_FORM_INPUT_NUMBER);
mSelectionCached = true;
mSelectionProperties = aProps;
}
bool IsSelectionCached() const
{
Expand Down
79 changes: 25 additions & 54 deletions dom/html/HTMLTextAreaElement.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ HTMLTextAreaElement::SetValue(const nsAString& aValue)

nsresult rv =
SetValueInternal(aValue, nsTextEditorState::eSetValue_ByContent |
nsTextEditorState::eSetValue_Notify);
nsTextEditorState::eSetValue_Notify |
nsTextEditorState::eSetValue_MoveCursorToEnd);
NS_ENSURE_SUCCESS(rv, rv);

if (mFocusedValue.Equals(currentValue)) {
Expand All @@ -369,7 +370,8 @@ HTMLTextAreaElement::SetUserInput(const nsAString& aValue)
{
return SetValueInternal(aValue,
nsTextEditorState::eSetValue_BySetUserInput |
nsTextEditorState::eSetValue_Notify);
nsTextEditorState::eSetValue_Notify|
nsTextEditorState::eSetValue_MoveCursorToEnd);
}

NS_IMETHODIMP
Expand Down Expand Up @@ -700,14 +702,7 @@ Nullable<uint32_t>
HTMLTextAreaElement::GetSelectionStart(ErrorResult& aError)
{
int32_t selStart, selEnd;
nsresult rv = GetSelectionRange(&selStart, &selEnd);

if (NS_FAILED(rv) && mState.IsSelectionCached()) {
return Nullable<uint32_t>(mState.GetSelectionProperties().GetStart());
}
if (NS_FAILED(rv)) {
aError.Throw(rv);
}
GetSelectionRange(&selStart, &selEnd, aError);
return Nullable<uint32_t>(selStart);
}

Expand All @@ -732,9 +727,8 @@ HTMLTextAreaElement::SetSelectionStart(const Nullable<uint32_t>& aSelectionStart
return;
}
int32_t start, end;
rv = GetSelectionRange(&start, &end);
if (NS_FAILED(rv)) {
aError.Throw(rv);
GetSelectionRange(&start, &end, aError);
if (aError.Failed()) {
return;
}
start = selStart;
Expand All @@ -751,14 +745,7 @@ Nullable<uint32_t>
HTMLTextAreaElement::GetSelectionEnd(ErrorResult& aError)
{
int32_t selStart, selEnd;
nsresult rv = GetSelectionRange(&selStart, &selEnd);

if (NS_FAILED(rv) && mState.IsSelectionCached()) {
return Nullable<uint32_t>(mState.GetSelectionProperties().GetEnd());
}
if (NS_FAILED(rv)) {
aError.Throw(rv);
}
GetSelectionRange(&selStart, &selEnd, aError);
return Nullable<uint32_t>(selEnd);
}

Expand All @@ -783,9 +770,8 @@ HTMLTextAreaElement::SetSelectionEnd(const Nullable<uint32_t>& aSelectionEnd,
return;
}
int32_t start, end;
rv = GetSelectionRange(&start, &end);
if (NS_FAILED(rv)) {
aError.Throw(rv);
GetSelectionRange(&start, &end, aError);
if (aError.Failed()) {
return;
}
end = selEnd;
Expand All @@ -798,16 +784,12 @@ HTMLTextAreaElement::SetSelectionEnd(const Nullable<uint32_t>& aSelectionEnd,
}
}

NS_IMETHODIMP
void
HTMLTextAreaElement::GetSelectionRange(int32_t* aSelectionStart,
int32_t* aSelectionEnd)
int32_t* aSelectionEnd,
ErrorResult& aRv)
{
// Flush frames, because our editor state will want to work with the frame.
if (IsInComposedDoc()) {
GetComposedDoc()->FlushPendingNotifications(FlushType::Frames);
}

return mState.GetSelectionRange(aSelectionStart, aSelectionEnd);
return mState.GetSelectionRange(aSelectionStart, aSelectionEnd, aRv);
}

static void
Expand Down Expand Up @@ -878,10 +860,12 @@ HTMLTextAreaElement::SetSelectionDirection(const nsAString& aDirection,
}

int32_t start, end;
nsresult rv = GetSelectionRange(&start, &end);
if (NS_SUCCEEDED(rv)) {
rv = SetSelectionRange(start, end, aDirection);
GetSelectionRange(&start, &end, aError);
if (aError.Failed()) {
return;
}

nsresult rv = SetSelectionRange(start, end, aDirection);
if (NS_FAILED(rv)) {
aError.Throw(rv);
}
Expand Down Expand Up @@ -937,13 +921,9 @@ HTMLTextAreaElement::SetRangeText(const nsAString& aReplacement,
ErrorResult& aRv)
{
int32_t start, end;
aRv = GetSelectionRange(&start, &end);
GetSelectionRange(&start, &end, aRv);
if (aRv.Failed()) {
if (mState.IsSelectionCached()) {
start = mState.GetSelectionProperties().GetStart();
end = mState.GetSelectionProperties().GetEnd();
aRv = NS_OK;
}
return;
}

SetRangeText(aReplacement, start, end, mozilla::dom::SelectionMode::Preserve,
Expand Down Expand Up @@ -975,13 +955,9 @@ HTMLTextAreaElement::SetRangeText(const nsAString& aReplacement,
}

if (aSelectionStart == -1 && aSelectionEnd == -1) {
aRv = GetSelectionRange(&aSelectionStart, &aSelectionEnd);
GetSelectionRange(&aSelectionStart, &aSelectionEnd, aRv);
if (aRv.Failed()) {
if (mState.IsSelectionCached()) {
aSelectionStart = mState.GetSelectionProperties().GetStart();
aSelectionEnd = mState.GetSelectionProperties().GetEnd();
aRv = NS_OK;
}
return;
}
}

Expand Down Expand Up @@ -1041,17 +1017,12 @@ HTMLTextAreaElement::SetRangeText(const nsAString& aReplacement,
nsresult
HTMLTextAreaElement::Reset()
{
nsresult rv;

// To get the initial spellchecking, reset value to
// empty string before setting the default value.
rv = SetValue(EmptyString());
NS_ENSURE_SUCCESS(rv, rv);
nsAutoString resetVal;
GetDefaultValue(resetVal);
SetValueChanged(false);

rv = SetValueInternal(resetVal, nsTextEditorState::eSetValue_Internal);
nsresult rv = SetValueInternal(resetVal,
nsTextEditorState::eSetValue_Internal);
NS_ENSURE_SUCCESS(rv, rv);

return NS_OK;
Expand Down
5 changes: 3 additions & 2 deletions dom/html/HTMLTextAreaElement.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,9 @@ class HTMLTextAreaElement final : public nsGenericHTMLFormElementWithState,
NS_IMETHOD_(void) InitializeKeyboardEventListeners() override;
NS_IMETHOD_(void) OnValueChanged(bool aNotify, bool aWasInteractiveUserChange) override;
NS_IMETHOD_(bool) HasCachedSelection() override;
NS_IMETHOD GetSelectionRange(int32_t* aSelectionStart,
int32_t* aSelectionEnd) override;
virtual void GetSelectionRange(int32_t* aSelectionStart,
int32_t* aSelectionEnd,
ErrorResult& aRv) override;


// nsIContent
Expand Down
Loading

0 comments on commit 77e7a55

Please sign in to comment.