commit ced0b032f23ae25ed6c362c81892547b477a2ea7
parent 90f9d2b12fe5a6e2408b0caf5bfb3109f6cd101e
Author: Masayuki Nakano <masayuki@d-toybox.com>
Date: Thu, 18 Dec 2025 00:04:30 +0000
Bug 2006520 - Make `TextInputListener::OnEditActionHandled` independent from `mFrame` r=emilio
It's called by `TextEditor` after (maybe) modifying the value before
dispatching `input` event.
First, it updates the enabled state of the undo/redo commands. However,
it's managed by the window. Therefore, it does not make sense to depend
on the frame.
Then, it updates the various state of the related objects. For making
them work with destroyed `TextEditor`, this patch makes `TextEditor`
keep storing the latest value with caching the `Text` while it's
handling something.
Differential Revision: https://phabricator.services.mozilla.com/D276790
Diffstat:
5 files changed, 120 insertions(+), 57 deletions(-)
diff --git a/dom/html/TextControlState.cpp b/dom/html/TextControlState.cpp
@@ -991,29 +991,21 @@ TextInputListener::HandleEvent(Event* aEvent) {
}
nsresult TextInputListener::OnEditActionHandled(TextEditor& aTextEditor) {
- if (mFrame) {
- // XXX Do we still need this or can we just remove the mFrame and
- // frame.IsAlive() conditions below?
- AutoWeakFrame weakFrame = mFrame;
-
- // Update the undo / redo menus
- //
- size_t numUndoItems = aTextEditor.NumberOfUndoItems();
- size_t numRedoItems = aTextEditor.NumberOfRedoItems();
- if ((numUndoItems && !mHadUndoItems) || (!numUndoItems && mHadUndoItems) ||
- (numRedoItems && !mHadRedoItems) || (!numRedoItems && mHadRedoItems)) {
- // Modify the menu if undo or redo items are different
- UpdateTextInputCommands(u"undo"_ns);
-
- mHadUndoItems = numUndoItems != 0;
- mHadRedoItems = numRedoItems != 0;
- }
+ // Update the undo / redo menus
+ //
+ size_t numUndoItems = aTextEditor.NumberOfUndoItems();
+ size_t numRedoItems = aTextEditor.NumberOfRedoItems();
+ if ((numUndoItems && !mHadUndoItems) || (!numUndoItems && mHadUndoItems) ||
+ (numRedoItems && !mHadRedoItems) || (!numRedoItems && mHadRedoItems)) {
+ // Modify the menu if undo or redo items are different
+ UpdateTextInputCommands(u"undo"_ns);
- if (weakFrame.IsAlive()) {
- HandleValueChanged(aTextEditor);
- }
+ mHadUndoItems = numUndoItems != 0;
+ mHadRedoItems = numRedoItems != 0;
}
+ HandleValueChanged(aTextEditor);
+
return mTextControlState ? mTextControlState->OnEditActionHandled() : NS_OK;
}
@@ -1038,11 +1030,7 @@ void TextInputListener::HandleValueChanged(TextEditor& aTextEditor) {
nsresult TextInputListener::UpdateTextInputCommands(
const nsAString& aCommandsToUpdate) {
- nsIContent* content = mFrame->GetContent();
- if (NS_WARN_IF(!content)) {
- return NS_ERROR_FAILURE;
- }
- nsCOMPtr<Document> doc = content->GetComposedDoc();
+ nsCOMPtr<Document> doc = mTxtCtrlElement->GetComposedDoc();
if (NS_WARN_IF(!doc)) {
return NS_ERROR_FAILURE;
}
diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp
@@ -6594,6 +6594,7 @@ EditorBase::AutoEditActionDataSetter::AutoEditActionDataSetter(
mSelection = mParentData->mSelection;
MOZ_ASSERT(!mSelection ||
(mSelection->GetType() == SelectionType::eNormal));
+ mTextNode = mParentData->mTextNode;
// If we're not editing something, we should inherit the parent's edit
// action. This may occur if creator or its callee use public methods which
@@ -6625,6 +6626,16 @@ EditorBase::AutoEditActionDataSetter::AutoEditActionDataSetter(
if (NS_WARN_IF(!mSelection)) {
return;
}
+ // Although we shouldn't have had the cached Text yet because we're the
+ // topmost instance of AutoEditActionDataSetter and we'll register this to
+ // aEditorBase below. However, for clarifying, let's explicitly ignore the
+ // cached Text. Additionally, this may be called for initializing
+ // aEditorBase too. Therefore, we need to avoid the assertions in
+ // GetTextNode() so that we need to check whether the editor is initialized.
+ mTextNode = mEditorBase.IsTextEditor() && mEditorBase.mInitSucceeded
+ ? mEditorBase.AsTextEditor()->GetTextNode(
+ TextEditor::IgnoreTextNodeCache::Yes)
+ : nullptr;
MOZ_ASSERT(mSelection->GetType() == SelectionType::eNormal);
@@ -6690,6 +6701,19 @@ EditorBase::AutoEditActionDataSetter::~AutoEditActionDataSetter() {
"mTopLevelEditSubActionData.mSelectedRange should've been cleared");
}
+void EditorBase::AutoEditActionDataSetter::OnEditorInitialized() {
+ if (mEditorWasDestroyedDuringHandlingEditAction) {
+ mEditorWasReinitialized = true;
+ }
+ if (mEditorBase.IsTextEditor()) {
+ mTextNode = mEditorBase.AsTextEditor()->GetTextNode(
+ TextEditor::IgnoreTextNodeCache::Yes);
+ }
+ if (mParentData) {
+ mParentData->OnEditorInitialized();
+ }
+}
+
void EditorBase::AutoEditActionDataSetter::UpdateSelectionCache(
Selection& aSelection) {
MOZ_ASSERT(aSelection.GetType() == SelectionType::eNormal);
diff --git a/editor/libeditor/EditorBase.h b/editor/libeditor/EditorBase.h
@@ -1127,6 +1127,11 @@ class EditorBase : public nsIEditor,
return *mSelection;
}
+ Text* GetCachedTextNode() const {
+ MOZ_ASSERT(mEditorBase.IsTextEditor());
+ return mTextNode;
+ }
+
nsIPrincipal* GetPrincipal() const { return mPrincipal; }
EditAction GetEditAction() const { return mEditAction; }
@@ -1223,14 +1228,7 @@ class EditorBase : public nsIEditor,
mParentData->OnEditorDestroy();
}
}
- void OnEditorInitialized() {
- if (mEditorWasDestroyedDuringHandlingEditAction) {
- mEditorWasReinitialized = true;
- }
- if (mParentData) {
- mParentData->OnEditorInitialized();
- }
- }
+ void OnEditorInitialized();
/**
* Return true if the editor was destroyed at least once while the
* EditAction is being handled. Note that the editor may have already been
@@ -1436,6 +1434,11 @@ class EditorBase : public nsIEditor,
RefPtr<Selection> mSelection;
nsTArray<OwningNonNull<Selection>> mRetiredSelections;
+ // mTextNode is the text node if and only if the instance is TextEditor.
+ // This is set when the instance is created and updated when the TextEditor
+ // is reinitialized with the new native anonymous subtree.
+ RefPtr<Text> mTextNode;
+
// True if the selection was created by doubleclicking a word.
bool mSelectionCreatedByDoubleclick{false};
@@ -1594,6 +1597,22 @@ class EditorBase : public nsIEditor,
return mEditActionData->SelectionRef();
}
+ // Return the Text if and only if we're a TextEditor instance. It's cached
+ // while we're handling an edit action, so, this stores the latest value even
+ // after we have been destroyed.
+ Text* GetCachedTextNode() {
+ MOZ_ASSERT(IsTextEditor());
+ return mEditActionData ? mEditActionData->GetCachedTextNode() : nullptr;
+ }
+
+ // Return the Text if and only if we're a TextEditor instance. It's cached
+ // while we're handling an edit action, so, this stores the latest value even
+ // after we have been destroyed.
+ const Text* GetCachedTextNode() const {
+ MOZ_ASSERT(IsTextEditor());
+ return const_cast<EditorBase*>(this)->GetCachedTextNode();
+ }
+
nsIPrincipal* GetEditActionPrincipal() const {
MOZ_ASSERT(mEditActionData);
return mEditActionData->GetPrincipal();
diff --git a/editor/libeditor/TextEditor.cpp b/editor/libeditor/TextEditor.cpp
@@ -460,12 +460,27 @@ NS_IMETHODIMP TextEditor::InsertLineBreak() {
}
nsresult TextEditor::ComputeTextValue(nsAString& aString) const {
- Element* anonymousDivElement = GetRoot();
- if (NS_WARN_IF(!anonymousDivElement)) {
- return NS_ERROR_NOT_INITIALIZED;
+ // This is a public method so that anytime this may be called, e.g., before
+ // initialized or after destroyed. However, GetTextNode() is an internal API
+ // for friend classes and internal use. Therefore, it asserts the conditions
+ // whether the anonymous subtree is available. Therefore, this method needs
+ // to get the Text with the anonymous <div>.
+ const Element* const anonymousDivElement = GetRoot();
+ if (MOZ_UNLIKELY(!anonymousDivElement)) {
+ // If we're temporarily destroyed while we're handling something, we can get
+ // the value from the cached text node which was maybe modified by us. This
+ // helps TextControlState and TextInputListener to notify the text control
+ // element of value changes, etc, at UnbindFromFrame().
+ const Text* const cachedTextNode = GetCachedTextNode();
+ if (NS_WARN_IF(!cachedTextNode)) {
+ return NS_ERROR_NOT_INITIALIZED;
+ }
+ cachedTextNode->GetData(aString);
+ return NS_OK;
}
- auto* text = Text::FromNodeOrNull(anonymousDivElement->GetFirstChild());
+ const auto* const text =
+ Text::FromNodeOrNull(anonymousDivElement->GetFirstChild());
if (MOZ_UNLIKELY(!text)) {
MOZ_ASSERT_UNREACHABLE("how?");
return NS_ERROR_UNEXPECTED;
@@ -583,33 +598,42 @@ already_AddRefed<Element> TextEditor::GetInputEventTargetElement() const {
}
bool TextEditor::IsEmpty() const {
- // This is a public method. Therefore, it might have not been initialized yet
- // when this is called. Let's return true in such case, but warn it because
- // it may return different value than actual value which is stored by the
- // text control element.
- MOZ_ASSERT_IF(mInitSucceeded, GetRoot());
- if (NS_WARN_IF(!GetRoot())) {
- NS_ASSERTION(false,
- "Make the root caller stop doing that before initializing or "
- "after destroying the TextEditor");
- return true;
+ // This is a public method so that anytime this may be called, e.g., before
+ // initialized or after destroyed. However, GetTextNode() is an internal API
+ // for friend classes and internal use. Therefore, it asserts the conditions
+ // whether the anonymous subtree is available. Therefore, this method needs
+ // to get the Text with the anonymous <div>.
+ if (MOZ_UNLIKELY(!mInitSucceeded)) {
+ // If we're temporarily destroyed while we're handling something, we can get
+ // the value from the cached text node which was maybe modified by us. This
+ // helps TextControlState and TextInputListener to notify the text control
+ // element of value changes, etc, at UnbindFromFrame().
+ const Text* const cachedTextNode = GetCachedTextNode();
+ return NS_WARN_IF(!cachedTextNode) || !cachedTextNode->TextDataLength();
}
const Text* const textNode = GetTextNode();
- MOZ_DIAGNOSTIC_ASSERT_IF(textNode,
- !Text::FromNodeOrNull(textNode->GetNextSibling()));
return !textNode || !textNode->TextDataLength();
}
NS_IMETHODIMP TextEditor::GetTextLength(uint32_t* aCount) {
MOZ_ASSERT(aCount);
-
- if (NS_WARN_IF(!GetRoot())) {
- return NS_ERROR_FAILURE;
+ // This is a public method so that anytime this may be called, e.g., before
+ // initialized or after destroyed. However, GetTextNode() is an internal API
+ // for friend classes and internal use. Therefore, it asserts the conditions
+ // whether the anonymous subtree is available. Therefore, this method needs
+ // to get the Text with the anonymous <div>.
+ if (MOZ_UNLIKELY(!mInitSucceeded)) {
+ // If we're temporarily destroyed while we're handling something, we can get
+ // the value from the cached text node which was maybe modified by us.
+ const Text* const textNode = GetCachedTextNode();
+ if (NS_WARN_IF(!textNode)) {
+ return NS_ERROR_FAILURE;
+ }
+ *aCount = textNode->TextDataLength();
+ return NS_OK;
}
const Text* const textNode = GetTextNode();
- MOZ_DIAGNOSTIC_ASSERT_IF(textNode,
- !Text::FromNodeOrNull(textNode->GetNextSibling()));
*aCount = textNode ? textNode->TextDataLength() : 0u;
return NS_OK;
}
diff --git a/editor/libeditor/TextEditor.h b/editor/libeditor/TextEditor.h
@@ -250,7 +250,14 @@ class TextEditor final : public EditorBase,
* Return the `Text` node in the anonymous <div>. Note that the anonymous
* <div> can have only one `Text` for the storage of the value of this editor.
*/
- dom::Text* GetTextNode() {
+ enum class IgnoreTextNodeCache : bool { No, Yes };
+ dom::Text* GetTextNode(
+ IgnoreTextNodeCache aIgnoreTextNodeCache = IgnoreTextNodeCache::No) {
+ if (aIgnoreTextNodeCache == IgnoreTextNodeCache::No) {
+ if (Text* const cachedTextNode = GetCachedTextNode()) {
+ return cachedTextNode;
+ }
+ }
MOZ_DIAGNOSTIC_ASSERT(GetRoot());
MOZ_DIAGNOSTIC_ASSERT(GetRoot()->GetFirstChild());
MOZ_DIAGNOSTIC_ASSERT(GetRoot()->GetFirstChild()->IsText());
@@ -259,8 +266,9 @@ class TextEditor final : public EditorBase,
}
return GetRoot()->GetFirstChild()->GetAsText();
}
- const dom::Text* GetTextNode() const {
- return const_cast<TextEditor*>(this)->GetTextNode();
+ const dom::Text* GetTextNode(IgnoreTextNodeCache aIgnoreTextNodeCache =
+ IgnoreTextNodeCache::No) const {
+ return const_cast<TextEditor*>(this)->GetTextNode(aIgnoreTextNodeCache);
}
protected: // May be called by friends.
