Bug 1566795 - part 5: Make the for loop of HTMLEditor::RemoveInlinePropertyInternal() partially selected text nodes r=m_kato

If selection range is not in **one** text node, `RemoveInlinePropertyInternal()`
collects target nodes with `SubtreeContentIterator`.  It only collects topmost
nodes which are **entirely** contained in the range (it's enough because their
descendants will be handled by `RemoveStyleInside()` recursively).

The reasons why it uses `SubtreeContentIterator` rather than
`PreContentIterator` must be:
1. Performance reason.
2. Assuming there are no multiple text nodes.
3. Not expects that user removes text node styles come from parent block.

The reason 2 is wrong because when removing a style, all browsers don't
join text nodes which was in removing element with adjacent text nodes.
(I.e., we cannot change this behavior for compatibility.)

The reason 3 is of course wrong we're struggling with this scenario.

Therefore, `RemoveInlinePropertyInternal()` needs to collect partially
selected text nodes by itself (if there are).  Then, we can merge the
single text node selected case with the `for` loop.

Differential Revision: https://phabricator.services.mozilla.com/D47864

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Masayuki Nakano 2019-10-08 09:24:47 +00:00
parent 3bbca5bdc3
commit 92ead719bd
2 changed files with 68 additions and 69 deletions

View File

@ -1581,51 +1581,51 @@ nsresult HTMLEditor::RemoveInlinePropertyInternal(nsAtom* aProperty,
return rv;
}
if (startOfRange.GetContainer() == endOfRange.GetContainer() &&
startOfRange.IsInTextNode()) {
// If parent block has the removing style, we should create `<span>`
// element to remove the style even in HTML mode since Chrome does it.
if (!CSSEditUtils::IsCSSEditableProperty(startOfRange.GetContainer(),
aProperty, aAttribute)) {
continue;
}
// The HTML style defined by aProperty/aAttribute has a CSS
// equivalence in this implementation for startOfRange.
if (!CSSEditUtils::IsCSSEquivalentToHTMLInlineStyleSet(
startOfRange.GetContainer(), aProperty, aAttribute,
EmptyString(), CSSEditUtils::eComputed)) {
continue;
}
// startOfRange's computed style indicates the CSS equivalence to the
// HTML style to remove is applied; but we found no element in the
// ancestors of startOfRange carrying specified styles; assume it
// comes from a rule and try to insert a span "inverting" the style
if (!CSSEditUtils::IsCSSInvertible(*aProperty, aAttribute)) {
continue;
}
SetInlinePropertyOnTextNode(
MOZ_KnownLive(*startOfRange.GetContainerAsText()),
startOfRange.Offset(), endOfRange.Offset(), *aProperty, aAttribute,
NS_LITERAL_STRING("-moz-editor-invert-value"));
if (NS_WARN_IF(Destroyed())) {
return NS_ERROR_EDITOR_DESTROYED;
}
continue;
}
// Collect editable nodes which are entirely contained in the range.
AutoTArray<OwningNonNull<nsIContent>, 64> arrayOfContents;
ContentSubtreeIterator subtreeIter;
if (NS_SUCCEEDED(subtreeIter.Init(range))) {
for (; !subtreeIter.IsDone(); subtreeIter.Next()) {
nsCOMPtr<nsINode> node = subtreeIter.GetCurrentNode();
if (NS_WARN_IF(!node)) {
return NS_ERROR_FAILURE;
}
if (node->IsContent() && IsEditable(node)) {
arrayOfContents.AppendElement(*node->AsContent());
if (startOfRange.GetContainer() == endOfRange.GetContainer() &&
startOfRange.IsInTextNode()) {
if (!IsEditable(startOfRange.GetContainer())) {
continue;
}
arrayOfContents.AppendElement(*startOfRange.GetContainerAsText());
} else if (startOfRange.IsInTextNode() && endOfRange.IsInTextNode() &&
startOfRange.GetContainer()->GetNextSibling() ==
endOfRange.GetContainer()) {
if (IsEditable(startOfRange.GetContainer())) {
arrayOfContents.AppendElement(*startOfRange.GetContainerAsText());
}
if (IsEditable(endOfRange.GetContainer())) {
arrayOfContents.AppendElement(*endOfRange.GetContainerAsText());
}
if (arrayOfContents.IsEmpty()) {
continue;
}
} else {
// Append first node if it's a text node but selected not entirely.
if (startOfRange.IsInTextNode() && !startOfRange.IsStartOfContainer() &&
IsEditable(startOfRange.GetContainer())) {
arrayOfContents.AppendElement(*startOfRange.GetContainerAsText());
}
// Append all entirely selected nodes.
ContentSubtreeIterator subtreeIter;
if (NS_SUCCEEDED(subtreeIter.Init(range))) {
for (; !subtreeIter.IsDone(); subtreeIter.Next()) {
nsCOMPtr<nsINode> node = subtreeIter.GetCurrentNode();
if (NS_WARN_IF(!node)) {
return NS_ERROR_FAILURE;
}
if (node->IsContent() && IsEditable(node)) {
arrayOfContents.AppendElement(*node->AsContent());
}
}
}
// Append last node if it's a text node but selected not entirely.
if (startOfRange.GetContainer() != endOfRange.GetContainer() &&
endOfRange.IsInTextNode() && !endOfRange.IsEndOfContainer() &&
IsEditable(endOfRange.GetContainer())) {
arrayOfContents.AppendElement(*endOfRange.GetContainerAsText());
}
}
for (auto& content : arrayOfContents) {
@ -1655,14 +1655,37 @@ nsresult HTMLEditor::RemoveInlinePropertyInternal(nsAtom* aProperty,
if (!CSSEditUtils::IsCSSInvertible(*aProperty, aAttribute)) {
continue;
}
DebugOnly<nsresult> rvIgnored = SetInlinePropertyOnNode(
content, *aProperty, aAttribute,
if (!content->IsText()) {
DebugOnly<nsresult> rvIgnored = SetInlinePropertyOnNode(
content, *aProperty, aAttribute,
NS_LITERAL_STRING("-moz-editor-invert-value"));
if (NS_WARN_IF(Destroyed())) {
return NS_ERROR_EDITOR_DESTROYED;
}
NS_WARNING_ASSERTION(NS_FAILED(rvIgnored),
"SetInlinePropertyOnNode() failed, but ignored");
continue;
}
// If current node is a text node, we need to create `<span>` element
// for it to overwrite parent style. Unfortunately, all browsers
// don't join text nodes when removing a style. Therefore, there
// may be multiple text nodes as adjacent siblings. That's the
// reason why we need to handle text nodes in this loop.
uint32_t startOffset =
content == startOfRange.GetContainer() ? startOfRange.Offset() : 0;
uint32_t endOffset = content == endOfRange.GetContainer()
? endOfRange.Offset()
: content->Length();
nsresult rv = SetInlinePropertyOnTextNode(
MOZ_KnownLive(*content->AsText()), startOffset, endOffset,
*aProperty, aAttribute,
NS_LITERAL_STRING("-moz-editor-invert-value"));
if (NS_WARN_IF(Destroyed())) {
return NS_ERROR_EDITOR_DESTROYED;
}
NS_WARNING_ASSERTION(NS_FAILED(rvIgnored),
"SetInlinePropertyOnNode() failed, but ignored");
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
}
}
}

View File

@ -167,30 +167,6 @@
[[["bold",""\]\] "<span style=\\"font-weight: 900\\">foo[barbaz</span>}" queryCommandState("bold") after]
expected: FAIL
[[["stylewithcss","true"\],["bold",""\]\] "{<h3>foobar\]baz</h3>" compare innerHTML]
expected: FAIL
[[["stylewithcss","true"\],["bold",""\]\] "{<h3>foobar\]baz</h3>" queryCommandState("bold") after]
expected: FAIL
[[["stylewithcss","false"\],["bold",""\]\] "{<h3>foobar\]baz</h3>" compare innerHTML]
expected: FAIL
[[["stylewithcss","false"\],["bold",""\]\] "{<h3>foobar\]baz</h3>" queryCommandState("bold") after]
expected: FAIL
[[["stylewithcss","true"\],["bold",""\]\] "<h3>foo[barbaz</h3>}" compare innerHTML]
expected: FAIL
[[["stylewithcss","true"\],["bold",""\]\] "<h3>foo[barbaz</h3>}" queryCommandState("bold") after]
expected: FAIL
[[["stylewithcss","false"\],["bold",""\]\] "<h3>foo[barbaz</h3>}" compare innerHTML]
expected: FAIL
[[["stylewithcss","false"\],["bold",""\]\] "<h3>foo[barbaz</h3>}" queryCommandState("bold") after]
expected: FAIL
[[["stylewithcss","true"\],["bold",""\]\] "<h3>[foobarbaz\]</h3>" compare innerHTML]
expected: FAIL