Bug 1780140 - Make `HTMLEditor::ClearStyleAt` clean up new empty inline elements which are not contain new text r=m_kato

Currently, `HTMLEditor` assumes that padding `<br>` element for empty last line
is outside of inline elements, but it may happen because of both:
* starting from bug 1778091, `HTMLEditor` move `<br>` element into new empty
inline elements at inserting new paragraph.
* web apps can put it into inline elements.

After splitting inline elements and which do not have meaningful content, users
cannot put caret into the empty inline elements so that the elements stay unless
delete around there.

For avoiding the leak due to meaningless elements, we should delete them at
splitting inline elements at inserting new text.

Note that Chrome does not pass the new tests of resetting ancestor bold style
because Chrome wraps the `<b>` with `<i>`, however, the `<i>` has odd
`style=""`.  Perhaps, the test framework should ignore it because it's not
important for the web-compatibility.

On the other hand, Chrome completely fails only the last testcase since it
unwraps the `<b>` from the last `<br>`, so the bold style which was applied by
the web app to the last `<br>` is lost.  This is not reasonable.

Differential Revision: https://phabricator.services.mozilla.com/D152616
This commit is contained in:
Masayuki Nakano 2022-07-27 22:51:18 +00:00
Родитель eccaaac4fa
Коммит bf2ccc8872
5 изменённых файлов: 210 добавлений и 26 удалений

Просмотреть файл

@ -628,7 +628,8 @@ nsresult HTMLEditor::OnEndHandlingTopLevelEditSubActionInternal() {
SelectionRef().GetFocusNode()->InclusiveAncestorsOfType<Element>()) {
if (!ancestor->IsHTMLElement() ||
!HTMLEditUtils::IsRemovableFromParentNode(*ancestor) ||
!HTMLEditUtils::IsEmptyInlineContent(*ancestor)) {
!HTMLEditUtils::IsEmptyInlineContainer(
*ancestor, {EmptyCheckOption::TreatSingleBRElementAsVisible})) {
break;
}
mostDistantEmptyInlineAncestor = ancestor;
@ -3126,7 +3127,8 @@ EditActionResult HTMLEditor::ChangeSelectedHardLinesToList(
for (auto& content : arrayOfContents) {
// if content is not a Break or empty inline, we're done
if (!content->IsHTMLElement(nsGkAtoms::br) &&
!HTMLEditUtils::IsEmptyInlineContent(content)) {
!HTMLEditUtils::IsEmptyInlineContainer(
content, {EmptyCheckOption::TreatSingleBRElementAsVisible})) {
bOnlyBreaks = false;
break;
}
@ -3266,7 +3268,8 @@ EditActionResult HTMLEditor::ChangeSelectedHardLinesToList(
// If current node is an empty inline node, just delete it.
if (EditorUtils::IsEditableContent(content, EditorType::HTML) &&
(content->IsHTMLElement(nsGkAtoms::br) ||
HTMLEditUtils::IsEmptyInlineContent(content))) {
HTMLEditUtils::IsEmptyInlineContainer(
content, {EmptyCheckOption::TreatSingleBRElementAsVisible}))) {
nsresult rv = DeleteNodeWithTransaction(*content);
if (NS_FAILED(rv)) {
NS_WARNING("EditorBase::DeleteNodeWithTransaction() failed");

Просмотреть файл

@ -1823,4 +1823,37 @@ size_t HTMLEditUtils::CollectChildren(
return numberOfFoundChildren;
}
// static
size_t HTMLEditUtils::CollectEmptyInlineContainerDescendants(
const nsINode& aNode,
nsTArray<OwningNonNull<nsIContent>>& aOutArrayOfContents,
const EmptyCheckOptions& aOptions) {
size_t numberOfFoundElements = 0;
for (Element* element = aNode.GetFirstElementChild(); element;) {
if (HTMLEditUtils::IsEmptyInlineContainer(*element, aOptions)) {
aOutArrayOfContents.AppendElement(*element);
numberOfFoundElements++;
nsIContent* nextContent = element->GetNextNonChildNode(&aNode);
element = nullptr;
for (; nextContent; nextContent = nextContent->GetNextNode(&aNode)) {
if (nextContent->IsElement()) {
element = nextContent->AsElement();
break;
}
}
continue;
}
nsIContent* nextContent = element->GetNextNode(&aNode);
element = nullptr;
for (; nextContent; nextContent = nextContent->GetNextNode(&aNode)) {
if (nextContent->IsElement()) {
element = nextContent->AsElement();
break;
}
}
}
return numberOfFoundElements;
}
} // namespace mozilla

Просмотреть файл

@ -455,14 +455,14 @@ class HTMLEditUtils final {
}
/**
* IsEmptyInlineContent() returns true if aContent is an inline node and it
* does not have meaningful content.
* IsEmptyInlineContainer() returns true if aContent is an inline element
* which can have children and does not have meaningful content.
*/
static bool IsEmptyInlineContent(const nsIContent& aContent) {
static bool IsEmptyInlineContainer(const nsIContent& aContent,
const EmptyCheckOptions& aOptions) {
return HTMLEditUtils::IsInlineElement(aContent) &&
HTMLEditUtils::IsContainerNode(aContent) &&
HTMLEditUtils::IsEmptyNode(
aContent, {EmptyCheckOption::TreatSingleBRElementAsVisible});
HTMLEditUtils::IsEmptyNode(aContent, aOptions);
}
/**
@ -500,7 +500,8 @@ class HTMLEditUtils final {
brElementHasFound = true;
continue;
}
if (!HTMLEditUtils::IsEmptyInlineContent(content)) {
if (!HTMLEditUtils::IsEmptyInlineContainer(
content, {EmptyCheckOption::TreatSingleBRElementAsVisible})) {
return false;
}
}
@ -2014,6 +2015,24 @@ class HTMLEditUtils final {
nsINode& aNode, nsTArray<OwningNonNull<nsIContent>>& aOutArrayOfContents,
size_t aIndexToInsertChildren, const CollectChildrenOptions& aOptions);
/**
* CollectEmptyInlineContainerDescendants() appends empty inline elements in
* aNode to aOutArrayOfContents. Although it's array of nsIContent, the
* instance will be elements.
*
* @param aNode The node whose descendants may have empty inline
* elements.
* @param aOutArrayOfContents [out] This method will append found descendants
* into this array.
* @param aOptions The option which element should be treated as
* empty.
* @return Number of found elements.
*/
static size_t CollectEmptyInlineContainerDescendants(
const nsINode& aNode,
nsTArray<OwningNonNull<nsIContent>>& aOutArrayOfContents,
const EmptyCheckOptions& aOptions);
private:
static bool CanNodeContain(nsHTMLTag aParentTagId, nsHTMLTag aChildTagId);
static bool IsContainerNode(nsHTMLTag aTagId);

Просмотреть файл

@ -3,6 +3,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 "ErrorList.h"
#include "HTMLEditor.h"
#include "EditAction.h"
@ -1097,6 +1098,11 @@ EditResult HTMLEditor::ClearStyleAt(const EditorDOMPoint& aPoint,
return EditResult(NS_ERROR_INVALID_ARG);
}
// TODO: We should rewrite this to stop unnecessary element creation and
// deleting it later because it causes the original element may be
// removed from the DOM tree even if same element is still in the
// DOM tree from point of view of users.
// First, split inline elements at the point.
// E.g., if aProperty is nsGkAtoms::b and `<p><b><i>a[]bc</i></b></p>`,
// we want to make it as `<p><b><i>a</i></b><b><i>bc</i></b></p>`.
@ -1119,7 +1125,10 @@ EditResult HTMLEditor::ClearStyleAt(const EditorDOMPoint& aPoint,
// If it did split nodes, but topmost ancestor inline element is split
// at start of it, we don't need the empty inline element. Let's remove
// it now.
// it now. Then, we'll get the following DOM tree if there is no "a" in the
// above case:
// <p><b><i>bc</i></b></p>
// ^^
if (splitResult.GetPreviousContent() &&
HTMLEditUtils::IsEmptyNode(
*splitResult.GetPreviousContent(),
@ -1183,23 +1192,38 @@ EditResult HTMLEditor::ClearStyleAt(const EditorDOMPoint& aPoint,
// selection so that just ignore it.
splitResultAtStartOfNextNode.IgnoreCaretPointSuggestion();
// Let's remove the next node if it becomes empty by splitting it.
// XXX Is this possible case without mutation event listener?
if (splitResultAtStartOfNextNode.Handled() &&
splitResultAtStartOfNextNode.GetNextContent() &&
HTMLEditUtils::IsEmptyNode(
*splitResultAtStartOfNextNode.GetNextContent(),
{EmptyCheckOption::TreatSingleBRElementAsVisible,
EmptyCheckOption::TreatListItemAsVisible,
EmptyCheckOption::TreatTableCellAsVisible})) {
// Delete next node if it's empty.
// MOZ_KnownLive(splitResultAtStartOfNextNode.GetNextContent()):
// It's grabbed by splitResultAtStartOfNextNode.
nsresult rv = DeleteNodeWithTransaction(
MOZ_KnownLive(*splitResultAtStartOfNextNode.GetNextContent()));
if (NS_FAILED(rv)) {
NS_WARNING("EditorBase::DeleteNodeWithTransaction() failed");
return EditResult(rv);
splitResultAtStartOfNextNode.GetNextContent()) {
// If the right inline elements are empty, we should remove them. E.g.,
// if the split point is at end of a text node (or end of an inline
// element), e.g., <div><b><i>abc[]</i></b></div>, then now, it's been
// changed to:
// <div><b><i>abc</i></b><b><i>[]</i></b><b><i></i></b></div>
// ^^^^^^^^^^^^^^
// We will change it to:
// <div><b><i>abc</i></b><b><i>[]</i></b></div>
// ^^
// And if it has only padding <br> element, we should move it into the
// previous <i> which will have new content.
bool seenBR = false;
if (HTMLEditUtils::IsEmptyNode(
*splitResultAtStartOfNextNode.GetNextContent(),
{EmptyCheckOption::TreatListItemAsVisible,
EmptyCheckOption::TreatTableCellAsVisible},
&seenBR)) {
// Delete next node if it's empty.
// MOZ_KnownLive(splitResultAtStartOfNextNode.GetNextContent()):
// It's grabbed by splitResultAtStartOfNextNode.
nsresult rv = DeleteNodeWithTransaction(
MOZ_KnownLive(*splitResultAtStartOfNextNode.GetNextContent()));
if (NS_FAILED(rv)) {
NS_WARNING("EditorBase::DeleteNodeWithTransaction() failed");
return EditResult(rv);
}
if (seenBR && !brElement) {
brElement = HTMLEditUtils::GetFirstBRElement(
*splitResultAtStartOfNextNode.GetNextContent()->AsElement());
}
}
}
@ -1227,6 +1251,7 @@ EditResult HTMLEditor::ClearStyleAt(const EditorDOMPoint& aPoint,
? firstLeafChildOfPreviousNode
: splitResultAtStartOfNextNode.GetPreviousContent(),
0);
// If the right node starts with a `<br>`, suck it out of right node and into
// the left node left node. This is so we you don't revert back to the
// previous style if you happen to click at the end of a line.
@ -1248,6 +1273,60 @@ EditResult HTMLEditor::ClearStyleAt(const EditorDOMPoint& aPoint,
NS_WARNING_ASSERTION(
rv != NS_SUCCESS_EDITOR_BUT_IGNORED_TRIVIAL_ERROR,
"MoveNodeResult::SuggestCaretPointTo() failed, but ignored");
if (splitResultAtStartOfNextNode.GetNextContent() &&
splitResultAtStartOfNextNode.GetNextContent()->IsInComposedDoc()) {
// If we split inline elements at immediately before <br> element which is
// the last visible content in the right element, we don't need the right
// element anymore. Otherwise, we'll create the following DOM tree:
// - <b>abc</b>{}<br><b></b>
// ^^^^^^^
// - <b><i>abc</i></b><i><br></i><b></b>
// ^^^^^^^
if (HTMLEditUtils::IsEmptyNode(
*splitResultAtStartOfNextNode.GetNextContent(),
{EmptyCheckOption::TreatSingleBRElementAsVisible,
EmptyCheckOption::TreatListItemAsVisible,
EmptyCheckOption::TreatTableCellAsVisible})) {
// MOZ_KnownLive because the result is grabbed by
// splitResultAtStartOfNextNode.
nsresult rv = DeleteNodeWithTransaction(
MOZ_KnownLive(*splitResultAtStartOfNextNode.GetNextContent()));
if (NS_FAILED(rv)) {
NS_WARNING("EditorBase::DeleteNodeWithTransaction() failed");
return EditResult(rv);
}
}
// If the next content has only one <br> element, there may be empty
// inline elements around it. We don't need them anymore because user
// cannot put caret into them. E.g., <b><i>abc[]<br></i><br></b> has
// been changed to <b><i>abc</i></b><i>{}<br></i><b><i></i><br></b> now.
// ^^^^^^^^^^^^^^^^^^
// We don't need the empty <i>.
else if (HTMLEditUtils::IsEmptyNode(
*splitResultAtStartOfNextNode.GetNextContent(),
{EmptyCheckOption::TreatListItemAsVisible,
EmptyCheckOption::TreatTableCellAsVisible})) {
AutoTArray<OwningNonNull<nsIContent>, 4> emptyInlineContainerElements;
HTMLEditUtils::CollectEmptyInlineContainerDescendants(
*splitResultAtStartOfNextNode.GetNextContent()->AsElement(),
emptyInlineContainerElements,
{EmptyCheckOption::TreatSingleBRElementAsVisible,
EmptyCheckOption::TreatListItemAsVisible,
EmptyCheckOption::TreatTableCellAsVisible});
for (const OwningNonNull<nsIContent>& emptyInlineContainerElement :
emptyInlineContainerElements) {
// MOZ_KnownLive(emptyInlineContainerElement) due to bug 1622253.
nsresult rv = DeleteNodeWithTransaction(
MOZ_KnownLive(emptyInlineContainerElement));
if (NS_FAILED(rv)) {
NS_WARNING("EditorBase::DeleteNodeWithTransaction() failed");
return EditResult(rv);
}
}
}
}
// Update the child.
pointToPutCaret.Set(pointToPutCaret.GetContainer(), 0);
}

Просмотреть файл

@ -2455,4 +2455,54 @@ var browserTests = [
"<div>abc<br></div><div><b>d</b><br></div>"],
[true,true,true],
{"bold":[false,true,"",false,true,""]}],
// Clearing style at end shouldn't leave empty inline element when there is
// <br> element in inline element
["<div><b>abc[]<br></b></div>",
[["styleWithCSS", "false"],["bold",""],["inserttext","d"]],
["<div><b>abc</b>d</div>",
"<div><b>abc</b>d<br></div>"],
[true,true,true],
{"bold":[false,true,"",false,false,""]}],
["<div><i><b>abc[]<br></b></i></div>",
[["styleWithCSS", "false"],["bold",""],["inserttext","d"]],
["<div><i><b>abc</b>d</i></div>",
"<div><i><b>abc</b>d<br></i></div>",
"<div><i><b>abc</b>d</i><br></div>"],
[true,true,true],
{"bold":[false,true,"",false,false,""]}],
["<div><i><b>abc[]</b><br></i></div>",
[["styleWithCSS", "false"],["bold",""],["inserttext","d"]],
["<div><i><b>abc</b>d</i></div>",
"<div><i><b>abc</b>d<br></i></div>"],
[true,true,true],
{"bold":[false,true,"",false,false,""]}],
["<div><b><i>abc[]<br></i></b></div>",
[["styleWithCSS", "false"],["bold",""],["inserttext","d"]],
["<div><b><i>abc</i></b><i>d</i></div>",
"<div><b><i>abc</i></b><i>d<br></i></div>",
"<div><b><i>abc</i></b><i>d</i><br></div>",
"<div><i><b>abc</b>d</i></div>",
"<div><i><b>abc</b>d<br></i></div>",
"<div><i><b>abc</b>d</i><br></div>"],
[true,true,true],
{"bold":[false,true,"",false,false,""]}],
["<div><b><i>abc[]</i><br></b></div>",
[["styleWithCSS", "false"],["bold",""],["inserttext","d"]],
["<div><b><i>abc</i></b><i>d</i></div>",
"<div><b><i>abc</i></b><i>d<br></i></div>",
"<div><b><i>abc</i></b><i>d</i><br></div>",
"<div><i><b>abc</b>d</i></div>",
"<div><i><b>abc</b>d<br></i></div>",
"<div><i><b>abc</b>d</i><br></div>"],
[true,true,true],
{"bold":[false,true,"",false,false,""]}],
// In this case, second line text should be keep bold style.
["<div><b><i>abc[]<br></i><br></b></div>",
[["styleWithCSS", "false"],["bold",""],["inserttext","d"]],
["<div><b><i>abc</i></b><i>d<br></i><b><br></b></div>",
"<div><i><b>abc</b>d<br></i><b><br></b></div>"],
[true,true,true],
{"bold":[false,true,"",false,false,""]}],
]