From f3c6a960d9a0ab39cb7dca33cfcd81869b649e1d Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 11 Jun 2025 17:03:31 +0000 Subject: [PATCH] Bug 1968843 - Make `HTMLEditor::HandleInsertText()` manage `Selection` if committing composition a=dmeehan DONTBUILD When committing composition string, `WhiteSpaceVisibilityKeeper::InsertOrUpdateCompositionString()` may replace commit string with normalized white-spaces. Therefore, if the last character is a white-space, it may be replaced with the other white-space (ASCII white-space vs. NBSP). Then, `Selection` collapsed after the last character will be moved to start of the replaced white-spaces. So, when committing composition, `HTMLEditor::HandleInsertText()` may need to change `Selection` by itself. Original Revision: https://phabricator.services.mozilla.com/D252278 Differential Revision: https://phabricator.services.mozilla.com/D253280 --- editor/libeditor/EditorBase.h | 5 + editor/libeditor/HTMLEditSubActionHandler.cpp | 116 ++++++++++++------ .../libeditor/WhiteSpaceVisibilityKeeper.cpp | 3 +- ...window_composition_text_querycontent.xhtml | 32 +++++ 4 files changed, 117 insertions(+), 39 deletions(-) diff --git a/editor/libeditor/EditorBase.h b/editor/libeditor/EditorBase.h index cc1df246b36c..8b149e377660 100644 --- a/editor/libeditor/EditorBase.h +++ b/editor/libeditor/EditorBase.h @@ -1759,6 +1759,11 @@ class EditorBase : public nsIEditor, return aPurpose == InsertTextFor::CompositionStart || aPurpose == InsertTextFor::CompositionStartAndEnd; } + [[nodiscard]] static bool InsertingTextForCommittingComposition( + InsertTextFor aPurpose) { + return aPurpose == InsertTextFor::CompositionEnd || + aPurpose == InsertTextFor::CompositionStartAndEnd; + } [[nodiscard]] static bool NothingToDoIfInsertingEmptyText( InsertTextFor aPurpose) { return aPurpose == InsertTextFor::NormalText || diff --git a/editor/libeditor/HTMLEditSubActionHandler.cpp b/editor/libeditor/HTMLEditSubActionHandler.cpp index 20743e60c55e..b28dfb632af2 100644 --- a/editor/libeditor/HTMLEditSubActionHandler.cpp +++ b/editor/libeditor/HTMLEditSubActionHandler.cpp @@ -4,6 +4,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include "EditorBase.h" #include "HTMLEditor.h" #include "HTMLEditorInlines.h" #include "HTMLEditorNestedClasses.h" @@ -1271,45 +1272,86 @@ Result HTMLEditor::HandleInsertText( return EditActionResult::HandledResult(); } - const auto compositionEndPoint = - GetLastIMESelectionEndPoint(); - Result replaceTextResult = - WhiteSpaceVisibilityKeeper::InsertOrUpdateCompositionString( - *this, aInsertionString, - compositionEndPoint.IsSet() - ? EditorDOMRange(pointToInsert, compositionEndPoint) - : EditorDOMRange(pointToInsert), - aPurpose); - if (MOZ_UNLIKELY(replaceTextResult.isErr())) { - NS_WARNING("WhiteSpaceVisibilityKeeper::ReplaceText() failed"); - return replaceTextResult.propagateErr(); + EditorDOMPoint endOfInsertedText; + { + AutoTrackDOMPoint trackPointToInsert(RangeUpdaterRef(), &pointToInsert); + const auto compositionEndPoint = + GetLastIMESelectionEndPoint(); + Result replaceTextResult = + WhiteSpaceVisibilityKeeper::InsertOrUpdateCompositionString( + *this, aInsertionString, + compositionEndPoint.IsSet() + ? EditorDOMRange(pointToInsert, compositionEndPoint) + : EditorDOMRange(pointToInsert), + aPurpose); + if (MOZ_UNLIKELY(replaceTextResult.isErr())) { + NS_WARNING("WhiteSpaceVisibilityKeeper::ReplaceText() failed"); + return replaceTextResult.propagateErr(); + } + InsertTextResult unwrappedReplaceTextResult = replaceTextResult.unwrap(); + nsresult rv = EnsureNoFollowingUnnecessaryLineBreak( + unwrappedReplaceTextResult.EndOfInsertedTextRef()); + if (NS_FAILED(rv)) { + NS_WARNING( + "HTMLEditor::EnsureNoFollowingUnnecessaryLineBreak() failed"); + return Err(rv); + } + endOfInsertedText = unwrappedReplaceTextResult.EndOfInsertedTextRef(); + if (InsertingTextForCommittingComposition(aPurpose)) { + // If we're committing the composition, + // WhiteSpaceVisibilityKeeper::InsertOrUpdateCompositionString() may + // replace the last character of the composition string when it's a + // white-space. Then, Selection will be moved before the last + // character. So, we need to adjust Selection here. + nsresult rv = unwrappedReplaceTextResult.SuggestCaretPointTo( + *this, {SuggestCaret::OnlyIfHasSuggestion, + SuggestCaret::OnlyIfTransactionsAllowedToDoIt, + SuggestCaret::AndIgnoreTrivialError}); + if (NS_FAILED(rv)) { + NS_WARNING("CaretPoint::SuggestCaretPointTo() failed"); + return Err(rv); + } + NS_WARNING_ASSERTION( + rv != NS_SUCCESS_EDITOR_BUT_IGNORED_TRIVIAL_ERROR, + "CaretPoint::SuggestCaretPoint() failed, but ignored"); + } else { + // CompositionTransaction should've set selection so that we should + // ignore caret suggestion. + unwrappedReplaceTextResult.IgnoreCaretPointSuggestion(); + } } - InsertTextResult unwrappedReplacedTextResult = replaceTextResult.unwrap(); - nsresult rv = EnsureNoFollowingUnnecessaryLineBreak( - unwrappedReplacedTextResult.EndOfInsertedTextRef()); - if (NS_FAILED(rv)) { - NS_WARNING("HTMLEditor::EnsureNoFollowingUnnecessaryLineBreak() failed"); - return Err(rv); - } - // CompositionTransaction should've set selection so that we should ignore - // caret suggestion. - unwrappedReplacedTextResult.IgnoreCaretPointSuggestion(); - const auto newCompositionStartPoint = - GetFirstIMESelectionStartPoint(); - const auto newCompositionEndPoint = - GetLastIMESelectionEndPoint(); - if (NS_WARN_IF(!newCompositionStartPoint.IsSet()) || - NS_WARN_IF(!newCompositionEndPoint.IsSet())) { - // Mutation event listener has changed the DOM tree... - return EditActionResult::HandledResult(); - } - rv = TopLevelEditSubActionDataRef().mChangedRange->SetStartAndEnd( - newCompositionStartPoint.ToRawRangeBoundary(), - newCompositionEndPoint.ToRawRangeBoundary()); - if (NS_FAILED(rv)) { - NS_WARNING("nsRange::SetStartAndEnd() failed"); - return Err(rv); + if (!InsertingTextForCommittingComposition(aPurpose)) { + const auto newCompositionStartPoint = + GetFirstIMESelectionStartPoint(); + const auto newCompositionEndPoint = + GetLastIMESelectionEndPoint(); + if (NS_WARN_IF(!newCompositionStartPoint.IsSet()) || + NS_WARN_IF(!newCompositionEndPoint.IsSet())) { + // Mutation event listener has changed the DOM tree... + return EditActionResult::HandledResult(); + } + nsresult rv = + TopLevelEditSubActionDataRef().mChangedRange->SetStartAndEnd( + newCompositionStartPoint.ToRawRangeBoundary(), + newCompositionEndPoint.ToRawRangeBoundary()); + if (NS_FAILED(rv)) { + NS_WARNING("nsRange::SetStartAndEnd() failed"); + return Err(rv); + } + } else { + if (NS_WARN_IF(!endOfInsertedText.IsSetAndValidInComposedDoc()) || + NS_WARN_IF(!pointToInsert.IsSetAndValidInComposedDoc())) { + return EditActionResult::HandledResult(); + } + nsresult rv = + TopLevelEditSubActionDataRef().mChangedRange->SetStartAndEnd( + pointToInsert.ToRawRangeBoundary(), + endOfInsertedText.ToRawRangeBoundary()); + if (NS_FAILED(rv)) { + NS_WARNING("nsRange::SetStartAndEnd() failed"); + return Err(rv); + } } return EditActionResult::HandledResult(); } diff --git a/editor/libeditor/WhiteSpaceVisibilityKeeper.cpp b/editor/libeditor/WhiteSpaceVisibilityKeeper.cpp index 782a6fa7e561..53124dd47447 100644 --- a/editor/libeditor/WhiteSpaceVisibilityKeeper.cpp +++ b/editor/libeditor/WhiteSpaceVisibilityKeeper.cpp @@ -3025,8 +3025,7 @@ WhiteSpaceVisibilityKeeper::InsertTextOrInsertOrUpdateCompositionString( } // If the composition is committed, we should normalize surrounding // white-spaces of the commit string. - if (aPurpose != InsertTextFor::CompositionEnd && - aPurpose != InsertTextFor::CompositionStartAndEnd) { + if (!EditorBase::InsertingTextForCommittingComposition(aPurpose)) { return insertOrReplaceTextResultOrError; } InsertTextResult insertOrReplaceTextResult = diff --git a/widget/tests/window_composition_text_querycontent.xhtml b/widget/tests/window_composition_text_querycontent.xhtml index fd31bb561697..e7036e833fde 100644 --- a/widget/tests/window_composition_text_querycontent.xhtml +++ b/widget/tests/window_composition_text_querycontent.xhtml @@ -11435,6 +11435,37 @@ function runCancelCompositionAfterCollapseWhileSpace() { ); } +function runCompositionOnlyWhiteSpace() { + const selection = windowOfContenteditable.getSelection(); + contenteditable.innerHTML = "

A

"; + contenteditable.focus(); + selection.collapse( + contenteditable.querySelector("[contenteditable] > p").firstChild, + "A".length + ); + const description = "runCompositionOnlyWhiteSpace"; + synthesizeCompositionChange({ + composition: {string: " ", clauses: [{length: 1, attr: COMPOSITION_ATTR_RAW_CLAUSE}]}, + }); + is( + contenteditable.innerHTML, + kNewWhiteSpaceNormalizerEnabled ? "

A 

" : "A
", + `${description}: Composing string " " should be inserted` + ); + synthesizeComposition({type: "compositioncommitasis", data: " "}); + is( + contenteditable.innerHTML, + kNewWhiteSpaceNormalizerEnabled ? "

A 

" : "

A

", + `${description}: Committing the white-space should not change the value` + ); + synthesizeKey("B"); + is( + contenteditable.innerHTML, + "

A B

", + `${description}: Typing "B" should cause inserting it after the white-space` + ); +} + async function runTest() { await SpecialPowers.pushPrefEnv({ @@ -11487,6 +11518,7 @@ async function runTest() runBug1675313Test(); runCommitCompositionWithSpaceKey(); runCancelCompositionAfterCollapseWhileSpace(); + runCompositionOnlyWhiteSpace(); runCompositionWithSelectionChange(); runCompositionWithClick(); runForceCommitTest();