corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1941520 - Make HTMLEditor::OnModifyDocument set top level edit sub-action before maintaining the white-space visibility r=m_kato

Browse Source 
When pasting text, a `paste` event is fired before a `beforeinput` event.
Therefore, the editor still does not have top level edit sub-action.  Therefore,
the DOM mutation caused by the web app will be handled by
`HTMLEditor::OnModifyDocument` immediately after removing the script blocker.
At this time, we may do:
* insert a padding `<br>` if something immediately after last input is removed
* replace a collapsible white-space if padding `<br>` is removed

Then, each handler sets the top level edit sub-action and the post-processor
will run immediately.  Then, especially in the latter case,
`WhiteSpaceVisibilityKeeper::NormalizeVisibleWhiteSpacesAt` will insert a
padding `<br>` again and restores the replaced NBSP to a collapsible
white-space unexpectedly.

Therefore, if web apps trying to normalize the pasted content with removing
the pasted nodes temporarily, it may cause entering an infinite loop.

This patch makes `HTMLEditor::OnModifyDocument` set edit sub-action for the
hacks to avoid running the post-processor.

Additionally, this touches `EditorBase::DoTransactionInternal` to avoid the
assertion failure of the new test.  The assertion failure indicates a logical
bug of our basic strategy.  However, we should not touch the big design change
for now.  (Anyway, the hack should be removed as soon as possible.)

Differential Revision: https://phabricator.services.mozilla.com/D234278
This commit is contained in:
Masayuki Nakano
2025-01-15 08:24:34 +00:00
parent a24b23bc19
commit 6f4cdf57e0
6 changed files with 169 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
editor/libeditor/EditAction.h
View File

@@ -503,6 +503,11 @@ enum class EditSubAction : int32_t {
// eCreatePaddingBRElementForEmptyEditor indicates to create a padding <br>
// element for empty editor.
eCreatePaddingBRElementForEmptyEditor,
// eMaintainWhiteSpaceVisibility indicates that editor updates `Text` nodes
// to make visible white-spaces keep visible even if web apps deletes padding
// <br>s for the preceding white-space.
eMaintainWhiteSpaceVisibility,
};
// You can use this macro as:

12
editor/libeditor/EditorBase.cpp
View File

@@ -851,8 +851,16 @@ nsresult EditorBase::GetSelection(SelectionType aSelectionType,
nsresult EditorBase::DoTransactionInternal(nsITransaction* aTransaction) {
MOZ_ASSERT(IsEditActionDataAvailable());
MOZ_ASSERT(!ShouldAlreadyHaveHandledBeforeInputEventDispatching(),
"beforeinput event hasn't been dispatched yet");
MOZ_ASSERT_IF(
// If the DOM is modified by a clipboard event handler,
// HTMLEditor::OnModifyDocument() may need to do some transactions before
// dispatching `beforeinput`.
// FIXME: It shouldn't happen, and I think that it should be done once
// before dispatching `input` event to hide the our editor hack from
// the event listeners.
GetEditAction() != EditAction::ePaste &&
GetEditAction() != EditAction::eCut,
!ShouldAlreadyHaveHandledBeforeInputEventDispatching());
if (mPlaceholderBatch && !mPlaceholderTransaction) {
MOZ_DIAGNOSTIC_ASSERT(mPlaceholderName);

1
editor/libeditor/EditorBase.h
View File

@@ -1279,6 +1279,7 @@ class EditorBase : public nsIEditor,
case EditSubAction::eRedo:
case EditSubAction::eComputeTextToOutput:
case EditSubAction::eCreatePaddingBRElementForEmptyEditor:
case EditSubAction::eMaintainWhiteSpaceVisibility:
case EditSubAction::eNone:
case EditSubAction::eReplaceHeadWithHTMLSource:
MOZ_ASSERT(aDirection == eNone);

8
editor/libeditor/HTMLEditSubActionHandler.cpp
View File

@@ -379,6 +379,14 @@ nsresult HTMLEditor::OnEndHandlingTopLevelEditSubAction() {
nsresult HTMLEditor::OnEndHandlingTopLevelEditSubActionInternal() {
MOZ_ASSERT(IsTopLevelEditSubActionDataAvailable());
// If we just maintained the DOM tree for consistent behavior even after
// web apps modified the DOM, we should not touch the DOM in this
// post-processor.
if (GetTopLevelEditSubAction() ==
EditSubAction::eMaintainWhiteSpaceVisibility) {
return NS_OK;
}
nsresult rv = EnsureSelectionInBodyOrDocumentElement();
if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
return NS_ERROR_EDITOR_DESTROYED;

156
editor/libeditor/HTMLEditor.cpp
View File

@@ -7760,85 +7760,101 @@ nsresult HTMLEditor::OnModifyDocument(const DocumentModifiedEvent& aRunner) {
// could destroy the editor
nsAutoScriptBlockerSuppressNodeRemoved scriptBlocker;
// If user typed a white-space at end of a text node recently, we should try
// to make it keep visible even after mutations caused by the web apps because
// only we use U+0020 as trailing visible white-space with <br>. Therefore,
// web apps may not take care of the white-space visibility.
// FIXME: Once we do this, the transaction should be merged to the last
// transaction for making an undoing deletes the inserted text too.
if (mLastCollapsibleWhiteSpaceAppendedTextNode &&
MOZ_LIKELY(
mLastCollapsibleWhiteSpaceAppendedTextNode->IsInComposedDoc() &&
mLastCollapsibleWhiteSpaceAppendedTextNode->IsEditable() &&
mLastCollapsibleWhiteSpaceAppendedTextNode->TextDataLength())) {
const auto atLastChar = EditorRawDOMPointInText::AtEndOf(
*mLastCollapsibleWhiteSpaceAppendedTextNode);
if (MOZ_LIKELY(atLastChar.IsPreviousCharCollapsibleASCIISpace())) {
if (const RefPtr<Element> editingHost = ComputeEditingHostInternal(
mLastCollapsibleWhiteSpaceAppendedTextNode,
LimitInBodyElement::No)) {
Result<CreateLineBreakResult, nsresult> insertPaddingBRResultOrError =
InsertPaddingBRElementIfNeeded(atLastChar.To<EditorDOMPoint>(),
eNoStrip, *editingHost);
if (MOZ_UNLIKELY(insertPaddingBRResultOrError.isErr())) {
if (insertPaddingBRResultOrError.inspectErr() ==
NS_ERROR_EDITOR_DESTROYED) {
{
// When this is called, there is no toplevel edit sub-action. Then,
// InsertNodeWithTransaction() or ReplaceTextWithTransaction() will set it.
// Then, OnEndHandlingTopLevelEditSubActionInternal() will call
// WhiteSpaceVisibilityKeeper::NormalizeVisibleWhiteSpacesAt() and may reset
// the hack here. Therefore, we need to make it sure that
// OnEndHandlingTopLevelEditSubActionInternal() does nothing later.
IgnoredErrorResult error;
AutoEditSubActionNotifier topLevelEditSubAction(
*this, EditSubAction::eMaintainWhiteSpaceVisibility, eNone, error);
NS_WARNING_ASSERTION(!error.Failed(),
"Failed to set the toplevel edit sub-action to "
"maintain white-space visibility, but ignored");
// If user typed a white-space at end of a text node recently, we should try
// to make it keep visible even after mutations caused by the web apps
// because only we use U+0020 as trailing visible white-space with <br>.
// Therefore, web apps may not take care of the white-space visibility.
// FIXME: Once we do this, the transaction should be merged to the last
// transaction for making an undoing deletes the inserted text too.
if (mLastCollapsibleWhiteSpaceAppendedTextNode &&
MOZ_LIKELY(
mLastCollapsibleWhiteSpaceAppendedTextNode->IsInComposedDoc() &&
mLastCollapsibleWhiteSpaceAppendedTextNode->IsEditable() &&
mLastCollapsibleWhiteSpaceAppendedTextNode->TextDataLength())) {
const auto atLastChar = EditorRawDOMPointInText::AtEndOf(
*mLastCollapsibleWhiteSpaceAppendedTextNode);
if (MOZ_LIKELY(atLastChar.IsPreviousCharCollapsibleASCIISpace())) {
if (const RefPtr<Element> editingHost = ComputeEditingHostInternal(
mLastCollapsibleWhiteSpaceAppendedTextNode,
LimitInBodyElement::No)) {
Result<CreateLineBreakResult, nsresult> insertPaddingBRResultOrError =
InsertPaddingBRElementIfNeeded(atLastChar.To<EditorDOMPoint>(),
eNoStrip, *editingHost);
if (MOZ_UNLIKELY(insertPaddingBRResultOrError.isErr())) {
if (insertPaddingBRResultOrError.inspectErr() ==
NS_ERROR_EDITOR_DESTROYED) {
NS_WARNING(
"HTMLEditor::InsertPaddingBRElementIfNeeded(nsIEditor::"
"eNoStrip) destroyed the editor");
return insertPaddingBRResultOrError.unwrapErr();
}
NS_WARNING(
"HTMLEditor::InsertPaddingBRElementIfNeeded(nsIEditor::"
"eNoStrip) destroyed the editor");
return insertPaddingBRResultOrError.unwrapErr();
"eNoStrip) failed, but ignored");
} else {
// We should not update selection for the mutation to maintain the
// white-space visibility.
insertPaddingBRResultOrError.unwrap().IgnoreCaretPointSuggestion();
}
NS_WARNING(
"HTMLEditor::InsertPaddingBRElementIfNeeded(nsIEditor::eNoStrip) "
"failed, but ignored");
} else {
// We should not update selection for the mutation to maintain the
// white-space visibility.
insertPaddingBRResultOrError.unwrap().IgnoreCaretPointSuggestion();
}
}
}
}
// If padding <br> element which made preceding collapsible ASCII white-space
// visible was removed by web app, we need to replace the white-space with
// an NBSP to make it keep visible until bug 503838 is fixed.
if (!aRunner.NewInvisibleWhiteSpacesRef().IsEmpty()) {
AutoSelectionRestorer restoreSelection(this);
bool doRestoreSelection = false;
for (const EditorDOMPointInText& atCollapsibleWhiteSpace :
aRunner.NewInvisibleWhiteSpacesRef()) {
if (!atCollapsibleWhiteSpace.IsInComposedDoc() ||
!atCollapsibleWhiteSpace.IsAtLastContent() ||
!HTMLEditUtils::IsSimplyEditableNode(
*atCollapsibleWhiteSpace.ContainerAs<Text>()) ||
!atCollapsibleWhiteSpace.IsCharCollapsibleASCIISpace()) {
continue;
// If padding <br> element which made preceding collapsible ASCII
// white-space visible was removed by web app, we need to replace the
// white-space with an NBSP to make it keep visible until bug 503838 is
// fixed.
if (!aRunner.NewInvisibleWhiteSpacesRef().IsEmpty()) {
AutoSelectionRestorer restoreSelection(this);
bool doRestoreSelection = false;
for (const EditorDOMPointInText& atCollapsibleWhiteSpace :
aRunner.NewInvisibleWhiteSpacesRef()) {
if (!atCollapsibleWhiteSpace.IsInComposedDoc() ||
!atCollapsibleWhiteSpace.IsAtLastContent() ||
!HTMLEditUtils::IsSimplyEditableNode(
*atCollapsibleWhiteSpace.ContainerAs<Text>()) ||
!atCollapsibleWhiteSpace.IsCharCollapsibleASCIISpace()) {
continue;
}
const Element* const editingHost =
atCollapsibleWhiteSpace.ContainerAs<Text>()->GetEditingHost();
MOZ_ASSERT(editingHost);
const WSScanResult nextThing =
WSRunScanner::ScanInclusiveNextVisibleNodeOrBlockBoundary(
editingHost,
atCollapsibleWhiteSpace.AfterContainer<EditorRawDOMPoint>(),
BlockInlineCheck::UseComputedDisplayStyle);
if (!nextThing.ReachedBlockBoundary()) {
continue;
}
Result<InsertTextResult, nsresult> replaceToNBSPResultOrError =
ReplaceTextWithTransaction(
MOZ_KnownLive(*atCollapsibleWhiteSpace.ContainerAs<Text>()),
atCollapsibleWhiteSpace.Offset(), 1u, u"\xA0"_ns);
if (MOZ_UNLIKELY(replaceToNBSPResultOrError.isErr())) {
NS_WARNING("HTMLEditor::ReplaceTextWithTransaction() failed");
continue;
}
doRestoreSelection = true;
replaceToNBSPResultOrError.unwrap().IgnoreCaretPointSuggestion();
}
const Element* const editingHost =
atCollapsibleWhiteSpace.ContainerAs<Text>()->GetEditingHost();
MOZ_ASSERT(editingHost);
const WSScanResult nextThing =
WSRunScanner::ScanInclusiveNextVisibleNodeOrBlockBoundary(
editingHost,
atCollapsibleWhiteSpace.AfterContainer<EditorRawDOMPoint>(),
BlockInlineCheck::UseComputedDisplayStyle);
if (!nextThing.ReachedBlockBoundary()) {
continue;
if (!doRestoreSelection) {
restoreSelection.Abort();
}
Result<InsertTextResult, nsresult> replaceToNBSPResultOrError =
ReplaceTextWithTransaction(
MOZ_KnownLive(*atCollapsibleWhiteSpace.ContainerAs<Text>()),
atCollapsibleWhiteSpace.Offset(), 1u, u"\xA0"_ns);
if (MOZ_UNLIKELY(replaceToNBSPResultOrError.isErr())) {
NS_WARNING("HTMLEditor::ReplaceTextWithTransaction() failed");
continue;
}
doRestoreSelection = true;
replaceToNBSPResultOrError.unwrap().IgnoreCaretPointSuggestion();
}
if (!doRestoreSelection) {
restoreSelection.Abort();
}
}

59
testing/web-platform/tests/editing/other/paste-text-after-collapsible-white-space-whose-container-remove-non-first-children.html Normal file
View File

@@ -0,0 +1,59 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Pasting text to editing host which tries to remove all inserted nodes shouldn't cause hangup</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/resources/testdriver-actions.js"></script>
<script src="../include/editor-test-utils.js"></script>
<script>
"use strict";
document.addEventListener("DOMContentLoaded", () => {
const copyTextContainer = document.querySelector("p");
const editingHost = document.querySelector("div[contenteditable]");
const utils = new EditorTestUtils(editingHost);
editingHost.addEventListener("paste", () => {
const fragment = document.createDocumentFragment();
let child;
while ((child = editingHost.childNodes[1])) {
fragment.appendChild(child);
}
});
promise_test(async () => {
copyTextContainer.innerHTML = "XYZ";
await test_driver.click(copyTextContainer);
getSelection().selectAllChildren(copyTextContainer);
await utils.sendCopyShortcutKey();
utils.setupEditingHost("abc def []<br>");
await utils.sendPasteShortcutKey();
assert_true(true); // Don't timeout
}, "Pasting text ends with a visible character");
promise_test(async () => {
copyTextContainer.innerHTML = "XYZ X";
await test_driver.click(copyTextContainer);
getSelection().setBaseAndExtent(
copyTextContainer.firstChild,
0,
copyTextContainer.firstChild,
"XYZ ".length
);
await utils.sendCopyShortcutKey();
utils.setupEditingHost("abc def []<br>");
await utils.sendPasteShortcutKey();
assert_true(true); // Don't timeout
}, "Pasting text ends with a collapsible white-space");
}, {once: true});
</script>
</head>
<body>
<p></p>
<div contenteditable></div>
</body>
</html>