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

Bug 1940377 - part 6: Make delete handlers use the new white-space normalizer r=m_kato

Browse Source 
Unfortunately, this patch becomes big because this includes multiple utility
methods to normalize white-spaces and touching a lot of places.

In some edge cases, this starts failing when the new white-space normalizer is
enabled.

```
FAIL execCommand("forwarddelete", false, ""): "a  [] |   b" - assert_equals: Modified text is wrong expected "a  " but got "a  "
FAIL execCommand("forwarddelete", false, ""): "a   [] |    b" - assert_equals: Modified text is wrong expected "a   " but got "a   "
FAIL execCommand("forwarddelete", false, ""): "a   []b|    c" - assert_equals: Modified text is wrong expected "a   " but got "a   "
FAIL execCommand("forwarddelete", false, ""): "a&nbsp;&nbsp;&nbsp;[]&nbsp;<span style=white-space:pre;>   </span>" - assert_equals: expected "a&nbsp;&nbsp;&nbsp;<span style=\"white-space:pre;\">   </span>" but got "a&nbsp; &nbsp;<span style=\"white-space:pre;\">   </span>"
```
In these failures, we normalize the trailing white-spaces of the preceding
`Text` too, but Chrome does not do that.  However, in some cases, they do
in similar cases.  Therefore, it's hard to align all cases due to the handling
order difference.  Additionally, these failure results are not odd.  So, these
new failures are acceptable.

```
FAIL execCommand("forwarddelete", false, ""): "<span>abc[]&nbsp;<span> &nbsp;def</span></span>" - assert_equals: expected "<span>abc<span>&nbsp; def</span></span>" but got "<span>abc<span> &nbsp;def</span></span>"
FAIL execCommand("forwarddelete", false, ""): "<span><span>abc[]&nbsp;</span> &nbsp;def</span>" - assert_equals: expected "<span><span>abc</span>&nbsp; def</span>" but got "<span><span>abc</span> &nbsp;def</span>"
FAIL execCommand("forwarddelete", false, ""): "<span>abc[]&nbsp;</span><span> &nbsp;def</span>" - assert_equals: expected "<span>abc</span><span>&nbsp; def</span>" but got "<span>abc</span><span> &nbsp;def</span>"
FAIL execCommand("forwarddelete", false, ""): "<span>abc[]&nbsp; </span><span>&nbsp;&nbsp;def</span>" - assert_equals: expected "<span>abc&nbsp;</span><span>&nbsp;&nbsp;def</span>" but got "<span>abc </span><span>&nbsp;&nbsp;def</span>"

```
In these failures, we don't normalize the leading white-spaces of the following
`Text`.  However, Chrome does so in some other cases.  So, it's hard to align
the behavior in these cases too.  The new normalizer ensures that the preceding
`Text` ends with a visible char.  Therefore, the following `Text` won't starts
with new invisible white-spaces.  Therefore, these failures are also acceptable.

```
FAIL execCommand("forwarddelete", false, ""): "<span style=white-space:pre;>  [] </span>&nbsp;&nbsp;&nbsp;a" - assert_equals: expected "<span style=\"white-space:pre;\">  </span>&nbsp; &nbsp;a" but got "<span style=\"white-space:pre;\">  </span>&nbsp;&nbsp;&nbsp;a"
```
In this failure, we don't normalize the following `Text` but Chrome does it.
However, I don't think we should do it because this updates the `Text` which
is preformatted.  Therefore, this is also acceptable.

So, I think that we can accept all new failures.

Differential Revision: https://phabricator.services.mozilla.com/D239468
This commit is contained in:
Masayuki Nakano
2025-03-08 22:31:56 +00:00
parent 1dc6c56a97
commit 2e3d9fe6d5
15 changed files with 2324 additions and 609 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
editor/libeditor/HTMLEditor.cpp
View File

@@ -4229,9 +4229,19 @@ Result<CaretPoint, nsresult> HTMLEditor::DeleteTextWithTransaction(
Result<InsertTextResult, nsresult> HTMLEditor::ReplaceTextWithTransaction(
dom::Text& aTextNode, const ReplaceWhiteSpacesData& aData) {
return ReplaceTextWithTransaction(aTextNode, aData.mReplaceStartOffset,
aData.ReplaceLength(),
aData.mNormalizedString);
Result<InsertTextResult, nsresult> insertTextResultOrError =
ReplaceTextWithTransaction(aTextNode, aData.mReplaceStartOffset,
aData.ReplaceLength(),
aData.mNormalizedString);
if (MOZ_UNLIKELY(insertTextResultOrError.isErr()) ||
aData.mNewOffsetAfterReplace > aTextNode.TextDataLength()) {
return insertTextResultOrError;
}
InsertTextResult insertTextResult = insertTextResultOrError.unwrap();
insertTextResult.IgnoreCaretPointSuggestion();
EditorDOMPoint pointToPutCaret(&aTextNode, aData.mNewOffsetAfterReplace);
return InsertTextResult(std::move(insertTextResult),
std::move(pointToPutCaret));
}
Result<InsertTextResult, nsresult> HTMLEditor::ReplaceTextWithTransaction(
@@ -5413,10 +5423,16 @@ nsresult HTMLEditor::CollapseAdjacentTextNodes(nsRange& aRange) {
continue;
}
Result<JoinNodesResult, nsresult> joinNodesResultOrError =
JoinNodesWithTransaction(MOZ_KnownLive(leftTextNode),
MOZ_KnownLive(rightTextNode));
StaticPrefs::editor_white_space_normalization_blink_compatible()
? JoinTextNodesWithNormalizeWhiteSpaces(
MOZ_KnownLive(leftTextNode), MOZ_KnownLive(rightTextNode))
: JoinNodesWithTransaction(MOZ_KnownLive(leftTextNode),
MOZ_KnownLive(rightTextNode));
if (MOZ_UNLIKELY(joinNodesResultOrError.isErr())) {
NS_WARNING("HTMLEditor::JoinNodesWithTransaction() failed");
NS_WARNING(
StaticPrefs::editor_white_space_normalization_blink_compatible()
? "HTMLEditor::JoinTextNodesWithNormalizeWhiteSpaces() failed"
: "HTMLEditor::JoinNodesWithTransaction() failed");
return joinNodesResultOrError.unwrapErr();
}
}