From d2afbdd254928536dc551839a12e96e72df650b4 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 28 Feb 2017 12:41:37 -0500 Subject: [PATCH] Bug 1342197 part 4. Move GetSelectionRange from nsTextControlFrame to the editor state. r=ehsan At this point, all this method does is ensure editor initialization and then ask the editor state for various information. Let's cut out the middleman. MozReview-Commit-ID: p491umScJO --- dom/html/HTMLInputElement.cpp | 56 +++++++++++++-------- dom/html/HTMLInputElement.h | 4 +- dom/html/HTMLTextAreaElement.cpp | 36 ++++++++------ dom/html/HTMLTextAreaElement.h | 5 +- dom/html/nsITextControlElement.h | 6 +++ dom/html/nsTextEditorState.cpp | 75 +++++++++++++++++++++++++++-- dom/html/nsTextEditorState.h | 6 +++ layout/forms/nsITextControlFrame.h | 3 -- layout/forms/nsTextControlFrame.cpp | 60 +++-------------------- layout/forms/nsTextControlFrame.h | 3 -- 10 files changed, 151 insertions(+), 103 deletions(-) diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index f39a06fa6699..fd71a296c33b 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -6607,17 +6607,30 @@ HTMLInputElement::GetFiles(nsIDOMFileList** aFileList) return NS_OK; } -nsresult +NS_IMETHODIMP HTMLInputElement::GetSelectionRange(int32_t* aSelectionStart, int32_t* aSelectionEnd) { - nsIFormControlFrame* formControlFrame = GetFormControlFrame(true); - nsITextControlFrame* textControlFrame = do_QueryFrame(formControlFrame); - if (textControlFrame) { - return textControlFrame->GetSelectionRange(aSelectionStart, aSelectionEnd); + // Flush frames, because our editor state will want to work with the frame. + if (IsInComposedDoc()) { + GetComposedDoc()->FlushPendingNotifications(FlushType::Frames); + } + if (!GetPrimaryFrame()) { + // Can we return a selection range anyway here, now that it lives on our + // state? In fact, could we make this behave more like + // GetSelectionDirection, in the sense of working even when we have no + // frame, by just delegating entirely to mState? And then, do we really + // need the flush? + return NS_ERROR_FAILURE; } - return NS_ERROR_FAILURE; + nsTextEditorState* state = GetEditorState(); + if (!state) { + // Not a text control. + return NS_ERROR_FAILURE; + } + + return state->GetSelectionRange(aSelectionStart, aSelectionEnd); } static void @@ -6642,28 +6655,29 @@ HTMLInputElement::GetSelectionDirection(nsAString& aDirection, ErrorResult& aRv) return; } - nsresult rv = NS_ERROR_FAILURE; nsIFormControlFrame* formControlFrame = GetFormControlFrame(true); - nsITextControlFrame* textControlFrame = do_QueryFrame(formControlFrame); - if (textControlFrame) { - nsITextControlFrame::SelectionDirection dir; - rv = textControlFrame->GetSelectionRange(nullptr, nullptr, &dir); - if (NS_SUCCEEDED(rv)) { - DirectionToName(dir, aDirection); - } + nsTextEditorState* state = GetEditorState(); + if (!state) { + aRv.Throw(NS_ERROR_FAILURE); + return; } - if (NS_FAILED(rv)) { - nsTextEditorState* state = GetEditorState(); - if (state && state->IsSelectionCached()) { - DirectionToName(state->GetSelectionProperties().GetDirection(), aDirection); + nsresult rv = NS_ERROR_FAILURE; + if (formControlFrame) { + nsITextControlFrame::SelectionDirection dir; + rv = state->GetSelectionDirection(&dir); + if (NS_SUCCEEDED(rv)) { + DirectionToName(dir, aDirection); return; } } - - if (NS_FAILED(rv)) { - aRv.Throw(rv); + + if (state->IsSelectionCached()) { + DirectionToName(state->GetSelectionProperties().GetDirection(), aDirection); + return; } + + aRv.Throw(rv); } NS_IMETHODIMP diff --git a/dom/html/HTMLInputElement.h b/dom/html/HTMLInputElement.h index effb453fb6c0..4dc62354dd50 100644 --- a/dom/html/HTMLInputElement.h +++ b/dom/html/HTMLInputElement.h @@ -240,6 +240,8 @@ public: 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; void GetDisplayFileName(nsAString& aFileName) const; @@ -956,8 +958,6 @@ protected: void SetIndeterminateInternal(bool aValue, bool aShouldInvalidate); - nsresult GetSelectionRange(int32_t* aSelectionStart, int32_t* aSelectionEnd); - /** * Called when an attribute is about to be changed */ diff --git a/dom/html/HTMLTextAreaElement.cpp b/dom/html/HTMLTextAreaElement.cpp index 0365d1b09a1d..3c6e66cde651 100644 --- a/dom/html/HTMLTextAreaElement.cpp +++ b/dom/html/HTMLTextAreaElement.cpp @@ -840,17 +840,24 @@ HTMLTextAreaElement::SetSelectionEnd(const Nullable& aSelectionEnd, } } -nsresult +NS_IMETHODIMP HTMLTextAreaElement::GetSelectionRange(int32_t* aSelectionStart, int32_t* aSelectionEnd) { - nsIFormControlFrame* formControlFrame = GetFormControlFrame(true); - nsITextControlFrame* textControlFrame = do_QueryFrame(formControlFrame); - if (textControlFrame) { - return textControlFrame->GetSelectionRange(aSelectionStart, aSelectionEnd); + // Flush frames, because our editor state will want to work with the frame. + if (IsInComposedDoc()) { + GetComposedDoc()->FlushPendingNotifications(FlushType::Frames); + } + if (!GetPrimaryFrame()) { + // Can we return a selection range anyway here, now that it lives on our + // state? In fact, could we make this behave more like + // GetSelectionDirection, in the sense of working even when we have no + // frame, by just delegating entirely to mState? And then, do we really + // need the flush? + return NS_ERROR_FAILURE; } - return NS_ERROR_FAILURE; + return mState.GetSelectionRange(aSelectionStart, aSelectionEnd); } static void @@ -880,22 +887,21 @@ HTMLTextAreaElement::GetSelectionDirection(nsAString& aDirection, ErrorResult& a { nsresult rv = NS_ERROR_FAILURE; nsIFormControlFrame* formControlFrame = GetFormControlFrame(true); - nsITextControlFrame* textControlFrame = do_QueryFrame(formControlFrame); - if (textControlFrame) { + if (formControlFrame) { nsITextControlFrame::SelectionDirection dir; - rv = textControlFrame->GetSelectionRange(nullptr, nullptr, &dir); + rv = mState.GetSelectionDirection(&dir); if (NS_SUCCEEDED(rv)) { DirectionToName(dir, aDirection); + return; } } - if (NS_FAILED(rv)) { - if (mState.IsSelectionCached()) { - DirectionToName(mState.GetSelectionProperties().GetDirection(), aDirection); - return; - } - aError.Throw(rv); + if (mState.IsSelectionCached()) { + DirectionToName(mState.GetSelectionProperties().GetDirection(), aDirection); + return; } + + aError.Throw(rv); } NS_IMETHODIMP diff --git a/dom/html/HTMLTextAreaElement.h b/dom/html/HTMLTextAreaElement.h index 833c5fdd46e5..37ae60ddfb41 100644 --- a/dom/html/HTMLTextAreaElement.h +++ b/dom/html/HTMLTextAreaElement.h @@ -109,6 +109,9 @@ public: 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; + // nsIContent virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent, @@ -347,8 +350,6 @@ protected: */ nsresult SetValueInternal(const nsAString& aValue, uint32_t aFlags); - nsresult GetSelectionRange(int32_t* aSelectionStart, int32_t* aSelectionEnd); - /** * Common method to call from the various mutation observer methods. * aContent is a content node that's either the one that changed or its diff --git a/dom/html/nsITextControlElement.h b/dom/html/nsITextControlElement.h index 40cb7058a540..dd52590f76c4 100644 --- a/dom/html/nsITextControlElement.h +++ b/dom/html/nsITextControlElement.h @@ -196,6 +196,12 @@ public: static already_AddRefed GetTextControlElementFromEditingHost(nsIContent* aHost); + + /** + * Get the selection range start and end points. + */ + NS_IMETHOD GetSelectionRange(int32_t* aSelectionStart, + int32_t* aSelectionEnd) = 0; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsITextControlElement, diff --git a/dom/html/nsTextEditorState.cpp b/dom/html/nsTextEditorState.cpp index a1cb1f70c75f..6b9c3153c28e 100644 --- a/dom/html/nsTextEditorState.cpp +++ b/dom/html/nsTextEditorState.cpp @@ -1549,6 +1549,73 @@ nsTextEditorState::SetSelectionProperties(nsTextEditorState::SelectionProperties } } +nsresult +nsTextEditorState::GetSelectionRange(int32_t* aSelectionStart, + int32_t* aSelectionEnd) +{ + MOZ_ASSERT(mBoundFrame, + "Caller didn't flush out frames and check for a frame?"); + MOZ_ASSERT(aSelectionStart); + MOZ_ASSERT(aSelectionEnd); + + // It's not clear that all the checks here are needed, but the previous + // version of this code in nsTextControlFrame was doing them, so we keep them + // for now. + + nsresult rv = mBoundFrame->EnsureEditorInitialized(); + NS_ENSURE_SUCCESS(rv, rv); + + nsISelectionController* selCon = GetSelectionController(); + NS_ENSURE_TRUE(selCon, NS_ERROR_FAILURE); + + nsCOMPtr selection; + rv = selCon->GetSelection(nsISelectionController::SELECTION_NORMAL, + getter_AddRefs(selection)); + NS_ENSURE_SUCCESS(rv, rv); + NS_ENSURE_TRUE(selection, NS_ERROR_FAILURE); + + dom::Selection* sel = selection->AsSelection(); + mozilla::dom::Element* root = GetRootNode(); + NS_ENSURE_STATE(root); + nsContentUtils::GetSelectionInTextControl(sel, root, + *aSelectionStart, *aSelectionEnd); + return NS_OK; +} + +nsresult +nsTextEditorState::GetSelectionDirection(nsITextControlFrame::SelectionDirection* aDirection) +{ + MOZ_ASSERT(mBoundFrame, + "Caller didn't flush out frames and check for a frame?"); + MOZ_ASSERT(aDirection); + + // It's not clear that all the checks here are needed, but the previous + // version of this code in nsTextControlFrame was doing them, so we keep them + // for now. + + nsresult rv = mBoundFrame->EnsureEditorInitialized(); + NS_ENSURE_SUCCESS(rv, rv); + + nsISelectionController* selCon = GetSelectionController(); + NS_ENSURE_TRUE(selCon, NS_ERROR_FAILURE); + + nsCOMPtr selection; + rv = selCon->GetSelection(nsISelectionController::SELECTION_NORMAL, + getter_AddRefs(selection)); + NS_ENSURE_SUCCESS(rv, rv); + NS_ENSURE_TRUE(selection, NS_ERROR_FAILURE); + + dom::Selection* sel = selection->AsSelection(); + nsDirection direction = sel->GetSelectionDirection(); + if (direction == eDirNext) { + *aDirection = nsITextControlFrame::eForward; + } else if (direction == eDirPrevious) { + *aDirection = nsITextControlFrame::eBackward; + } else { + NS_NOTREACHED("Invalid nsDirection enum value"); + } + return NS_OK; +} HTMLInputElement* nsTextEditorState::GetParentNumberControl(nsFrame* aFrame) const @@ -1621,7 +1688,7 @@ nsTextEditorState::UnbindFromFrame(nsTextControlFrame* aFrame) } // Save our selection state if needed. - // Note that nsTextControlFrame::GetSelectionRange attempts to initialize the + // Note that GetSelectionRange attempts to initialize the // editor before grabbing the range, and because this is not an acceptable // side effect for unbinding from a text control frame, we need to call // GetSelectionRange before calling DestroyEditor, and only if @@ -1636,13 +1703,15 @@ nsTextEditorState::UnbindFromFrame(nsTextControlFrame* aFrame) // parent control, because this text editor state will be destroyed // together with the native anonymous text control. SelectionProperties props; - mBoundFrame->GetSelectionRange(&start, &end, &direction); + GetSelectionRange(&start, &end); + GetSelectionDirection(&direction); props.SetStart(start); props.SetEnd(end); props.SetDirection(direction); number->SetSelectionProperties(props); } else { - mBoundFrame->GetSelectionRange(&start, &end, &direction); + GetSelectionRange(&start, &end); + GetSelectionDirection(&direction); mSelectionProperties.SetStart(start); mSelectionProperties.SetEnd(end); mSelectionProperties.SetDirection(direction); diff --git a/dom/html/nsTextEditorState.h b/dom/html/nsTextEditorState.h index 11494f155a75..b07de4842e2e 100644 --- a/dom/html/nsTextEditorState.h +++ b/dom/html/nsTextEditorState.h @@ -262,6 +262,12 @@ public: void WillInitEagerly() { mSelectionRestoreEagerInit = true; } bool HasNeverInitializedBefore() const { return !mEverInited; } + // Get the selection range start and end points in our text. + nsresult GetSelectionRange(int32_t* aSelectionStart, int32_t* aSelectionEnd); + + // Get the selection direction + nsresult GetSelectionDirection(nsITextControlFrame::SelectionDirection* aDirection); + void UpdateEditableState(bool aNotify) { if (mRootNode) { mRootNode->UpdateEditableState(aNotify); diff --git a/layout/forms/nsITextControlFrame.h b/layout/forms/nsITextControlFrame.h index ce46b66ff85f..59cad86f31ee 100644 --- a/layout/forms/nsITextControlFrame.h +++ b/layout/forms/nsITextControlFrame.h @@ -31,9 +31,6 @@ public: NS_IMETHOD SetSelectionRange(int32_t aSelectionStart, int32_t aSelectionEnd, SelectionDirection aDirection = eNone) = 0; - NS_IMETHOD GetSelectionRange(int32_t* aSelectionStart, - int32_t* aSelectionEnd, - SelectionDirection* aDirection = nullptr) = 0; NS_IMETHOD GetOwnedSelectionController(nsISelectionController** aSelCon) = 0; virtual nsFrameSelection* GetOwnedFrameSelection() = 0; diff --git a/layout/forms/nsTextControlFrame.cpp b/layout/forms/nsTextControlFrame.cpp index 9ce80e74c3e0..d10887d8aad7 100644 --- a/layout/forms/nsTextControlFrame.cpp +++ b/layout/forms/nsTextControlFrame.cpp @@ -916,7 +916,9 @@ nsTextControlFrame::SetSelectionStart(int32_t aSelectionStart) int32_t selStart = 0, selEnd = 0; - rv = GetSelectionRange(&selStart, &selEnd); + nsCOMPtr txtCtrl = do_QueryInterface(GetContent()); + MOZ_ASSERT(txtCtrl, "Content not a text control element"); + rv = txtCtrl->GetSelectionRange(&selStart, &selEnd); NS_ENSURE_SUCCESS(rv, rv); if (aSelectionStart > selEnd) { @@ -937,7 +939,9 @@ nsTextControlFrame::SetSelectionEnd(int32_t aSelectionEnd) int32_t selStart = 0, selEnd = 0; - rv = GetSelectionRange(&selStart, &selEnd); + nsCOMPtr txtCtrl = do_QueryInterface(GetContent()); + MOZ_ASSERT(txtCtrl, "Content not a text control element"); + rv = txtCtrl->GetSelectionRange(&selStart, &selEnd); NS_ENSURE_SUCCESS(rv, rv); if (aSelectionEnd < selStart) { @@ -1009,58 +1013,6 @@ nsTextControlFrame::OffsetToDOMPoint(int32_t aOffset, return NS_OK; } -NS_IMETHODIMP -nsTextControlFrame::GetSelectionRange(int32_t* aSelectionStart, - int32_t* aSelectionEnd, - SelectionDirection* aDirection) -{ - // make sure we have an editor - nsresult rv = EnsureEditorInitialized(); - NS_ENSURE_SUCCESS(rv, rv); - - if (aSelectionStart) { - *aSelectionStart = 0; - } - if (aSelectionEnd) { - *aSelectionEnd = 0; - } - if (aDirection) { - *aDirection = eNone; - } - - nsCOMPtr txtCtrl = do_QueryInterface(GetContent()); - NS_ASSERTION(txtCtrl, "Content not a text control element"); - nsISelectionController* selCon = txtCtrl->GetSelectionController(); - NS_ENSURE_TRUE(selCon, NS_ERROR_FAILURE); - nsCOMPtr selection; - rv = selCon->GetSelection(nsISelectionController::SELECTION_NORMAL, getter_AddRefs(selection)); - NS_ENSURE_SUCCESS(rv, rv); - NS_ENSURE_TRUE(selection, NS_ERROR_FAILURE); - - dom::Selection* sel = selection->AsSelection(); - if (aDirection) { - nsDirection direction = sel->GetSelectionDirection(); - if (direction == eDirNext) { - *aDirection = eForward; - } else if (direction == eDirPrevious) { - *aDirection = eBackward; - } else { - NS_NOTREACHED("Invalid nsDirection enum value"); - } - } - - if (!aSelectionStart || !aSelectionEnd) { - return NS_OK; - } - - mozilla::dom::Element* root = txtCtrl->GetRootEditorNode(); - NS_ENSURE_STATE(root); - nsContentUtils::GetSelectionInTextControl(sel, root, - *aSelectionStart, *aSelectionEnd); - - return NS_OK; -} - /////END INTERFACE IMPLEMENTATIONS ////NSIFRAME diff --git a/layout/forms/nsTextControlFrame.h b/layout/forms/nsTextControlFrame.h index 175e37cc715b..0ccbcb6531db 100644 --- a/layout/forms/nsTextControlFrame.h +++ b/layout/forms/nsTextControlFrame.h @@ -147,9 +147,6 @@ public: NS_IMETHOD SetSelectionRange(int32_t aSelectionStart, int32_t aSelectionEnd, SelectionDirection aDirection = eNone) override; - NS_IMETHOD GetSelectionRange(int32_t* aSelectionStart, - int32_t* aSelectionEnd, - SelectionDirection* aDirection = nullptr) override; NS_IMETHOD GetOwnedSelectionController(nsISelectionController** aSelCon) override; virtual nsFrameSelection* GetOwnedFrameSelection() override;