Bug 1789574 - part 1: Make `HTMLEditor` preserve order of inline style elements too r=m_kato

Currently, `HTMLEditor` preserves styled elements with the reversed order in
the loop.
https://searchfox.org/mozilla-central/rev/d7d5c89ee7bc70dec0fd846689ca030f94931393/editor/libeditor/HTMLEditSubActionHandler.cpp#8638-8658

I.e., if `<b><i></i></b>` is restored in the HTML styling mode, it will be
restored as `<i><b>...</b></i>`.  This blocks writing tests for bug 1789574.
Therefore, this should be fixed first.

(As I commented in the method, ideally we should store all inline elements and
restore all of them, but currently we don't need to do it.  It requires redesign
of the style cache, but it seems that we don't know web apps which is broken by
current behavior.  Therefore, I think that we should do it when we meet a
trouble with the behavior.)

Depends on D164519

Differential Revision: https://phabricator.services.mozilla.com/D164520
This commit is contained in:
Masayuki Nakano 2022-12-15 09:11:47 +00:00
Родитель 077fe4f24c
Коммит 4493e43e5d
5 изменённых файлов: 92 добавлений и 68 удалений

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

@ -9028,7 +9028,87 @@ nsresult HTMLEditor::GetInlineStyles(
MOZ_ASSERT(IsEditActionDataAvailable());
MOZ_ASSERT(aPendingStyleCacheArray.IsEmpty());
bool useCSS = IsCSSEnabled();
if (!IsCSSEnabled()) {
// In the HTML styling mode, we should preserve the order of inline styles
// specified with HTML elements, then, we can keep same order as original
// one when we create new elements to apply the styles at new place.
// XXX Currently, we don't preserve all inline parents, therefore, we cannot
// restore all inline elements as-is. Perhaps, we should store all
// inline elements with more details (e.g., all attributes), and store
// same elements. For example, web apps may give style as:
// em {
// font-style: italic;
// }
// em em {
// font-style: normal;
// font-weight: bold;
// }
// but we cannot restore the style as-is.
nsString value;
const bool givenElementIsEditable =
HTMLEditUtils::IsSimplyEditableNode(aElement);
auto NeedToAppend = [&](nsStaticAtom& aTagName, nsStaticAtom* aAttribute) {
if (mPendingStylesToApplyToNewContent->GetStyleState(
aTagName, aAttribute) != PendingStyleState::NotUpdated) {
return false; // The style has already been changed.
}
if (aPendingStyleCacheArray.Contains(aTagName, aAttribute)) {
return false; // Already preserved
}
return true;
};
for (Element* const inclusiveAncestor :
aElement.InclusiveAncestorsOfType<Element>()) {
if (HTMLEditUtils::IsBlockElement(*inclusiveAncestor) ||
(givenElementIsEditable &&
!HTMLEditUtils::IsSimplyEditableNode(*inclusiveAncestor))) {
break;
}
if (inclusiveAncestor->IsAnyOfHTMLElements(
nsGkAtoms::b, nsGkAtoms::i, nsGkAtoms::u, nsGkAtoms::s,
nsGkAtoms::strike, nsGkAtoms::tt, nsGkAtoms::em,
nsGkAtoms::strong, nsGkAtoms::dfn, nsGkAtoms::code,
nsGkAtoms::samp, nsGkAtoms::var, nsGkAtoms::cite, nsGkAtoms::abbr,
nsGkAtoms::acronym, nsGkAtoms::sub, nsGkAtoms::sup)) {
nsStaticAtom& tagName = const_cast<nsStaticAtom&>(
*inclusiveAncestor->NodeInfo()->NameAtom()->AsStatic());
if (NeedToAppend(tagName, nullptr)) {
aPendingStyleCacheArray.AppendElement(
PendingStyleCache(tagName, nullptr, EmptyString()));
}
continue;
}
if (inclusiveAncestor->IsHTMLElement(nsGkAtoms::font)) {
if (NeedToAppend(*nsGkAtoms::font, nsGkAtoms::face)) {
inclusiveAncestor->GetAttr(kNameSpaceID_None, nsGkAtoms::face, value);
if (!value.IsEmpty()) {
aPendingStyleCacheArray.AppendElement(
PendingStyleCache(*nsGkAtoms::font, nsGkAtoms::face, value));
value.Truncate();
}
}
if (NeedToAppend(*nsGkAtoms::font, nsGkAtoms::size)) {
inclusiveAncestor->GetAttr(kNameSpaceID_None, nsGkAtoms::size, value);
if (!value.IsEmpty()) {
aPendingStyleCacheArray.AppendElement(
PendingStyleCache(*nsGkAtoms::font, nsGkAtoms::size, value));
value.Truncate();
}
}
if (NeedToAppend(*nsGkAtoms::font, nsGkAtoms::color)) {
inclusiveAncestor->GetAttr(kNameSpaceID_None, nsGkAtoms::color,
value);
if (!value.IsEmpty()) {
aPendingStyleCacheArray.AppendElement(
PendingStyleCache(*nsGkAtoms::font, nsGkAtoms::color, value));
value.Truncate();
}
}
continue;
}
}
return NS_OK;
}
for (nsStaticAtom* property : {nsGkAtoms::b,
nsGkAtoms::i,
@ -9067,7 +9147,7 @@ nsresult HTMLEditor::GetInlineStyles(
nsString value; // Don't use nsAutoString here because it requires memcpy
// at creating new PendingStyleCache instance.
// Don't use CSS for <font size>, we don't support it usefully (bug 780035)
if (!useCSS || (property == nsGkAtoms::size)) {
if (property == nsGkAtoms::size) {
isSet = HTMLEditUtils::IsInlineStyleSetByElement(aElement, style, nullptr,
&value);
} else if (style.IsCSSEditable(aElement)) {

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

@ -105,8 +105,8 @@ class PendingStyleCache final {
class MOZ_STACK_CLASS AutoPendingStyleCacheArray final
: public AutoTArray<PendingStyleCache, 21> {
public:
index_type IndexOf(const nsStaticAtom& aTag,
const nsStaticAtom* aAttribute) const {
[[nodiscard]] index_type IndexOf(const nsStaticAtom& aTag,
const nsStaticAtom* aAttribute) const {
for (index_type index = 0; index < Length(); ++index) {
const PendingStyleCache& styleCache = ElementAt(index);
if (&styleCache.TagRef() == &aTag &&
@ -116,6 +116,11 @@ class MOZ_STACK_CLASS AutoPendingStyleCacheArray final
}
return NoIndex;
}
[[nodiscard]] bool Contains(const nsStaticAtom& aTag,
const nsStaticAtom* aAttribute) const {
return IndexOf(aTag, aAttribute) != NoIndex;
}
};
enum class PendingStyleState {

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

@ -330,9 +330,6 @@
[[["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc{<b><i>def</i></b>}ghi</div>" compare innerHTML]
expected: FAIL
[[["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc[<b><i>def\]</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["inserttext","d"\],["inserttext","e"\],["inserttext","f"\],["inserttext","g"\]\] "<div>abc[<b><i>def</i></b>g\]hi</div>" compare innerHTML]
expected: FAIL
@ -347,15 +344,3 @@
[[["inserttext","c"\],["inserttext","d"\],["inserttext","e"\]\] "<div>ab[c<b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["inserttext","d"\],["inserttext","e"\]\] "<div>abc[<b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["inserttext","d"\],["inserttext","e"\]\] "<div>abc{<b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["inserttext","d"\],["inserttext","e"\]\] "<div>abc[<s></s><b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["inserttext","d"\],["inserttext","e"\]\] "<div>abc{<s></s><b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL

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

@ -1144,27 +1144,12 @@
[[["forwarddelete",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc{<b><i>def</i></b>}ghi</div>" compare innerHTML]
expected: FAIL
[[["delete",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc<b><i>[def\]</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["forwarddelete",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc<b><i>[def\]</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["delete",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc[<b><i>def\]</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["forwarddelete",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc[<b><i>def\]</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["delete",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc<b><i>[def</i></b>\]ghi</div>" compare innerHTML]
expected: FAIL
[[["forwarddelete",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc<b><i>[def</i></b>\]ghi</div>" compare innerHTML]
expected: FAIL
[[["delete",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\],["inserttext","g"\]\] "<div>abc<b><i>[def</i></b>g\]hi</div>" compare innerHTML]
expected: FAIL
[[["forwarddelete",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\],["inserttext","g"\]\] "<div>abc<b><i>[def</i></b>g\]hi</div>" compare innerHTML]
expected: FAIL
@ -1183,12 +1168,6 @@
[[["insertparagraph",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc{<b><i>def</i></b>}ghi</div>" compare innerHTML]
expected: FAIL
[[["insertparagraph",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc<b><i>[def\]</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["insertparagraph",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc[<b><i>def\]</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["insertparagraph",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc<b><i>[def</i></b>\]ghi</div>" compare innerHTML]
expected: FAIL
@ -1210,9 +1189,6 @@
[[["forwarddelete",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc[<b><i><img src=\\"/img/lion.svg\\">def\]</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["insertlinebreak",""\],["inserttext","d"\],["inserttext","e"\]\] "<div>abc[<b><i><img src=\\"/img/lion.svg\\">de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["insertparagraph",""\],["inserttext","d"\],["inserttext","e"\],["inserttext","f"\]\] "<div>abc[<b><i><img src=\\"/img/lion.svg\\">def\]</i></b>ghi</div>" compare innerHTML]
expected: FAIL
@ -1225,9 +1201,6 @@
[[["forwarddelete",""\],["inserttext","c"\],["inserttext","d"\],["inserttext","e"\]\] "<div>ab[c<b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["insertlinebreak",""\],["inserttext","c"\],["inserttext","d"\],["inserttext","e"\]\] "<div>ab[c<b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["delete",""\],["inserttext","d"\],["inserttext","e"\]\] "<div>abc[<b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
@ -1240,12 +1213,6 @@
[[["forwarddelete",""\],["inserttext","d"\],["inserttext","e"\]\] "<div>abc{<b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["insertlinebreak",""\],["inserttext","d"\],["inserttext","e"\]\] "<div>abc[<b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["insertlinebreak",""\],["inserttext","d"\],["inserttext","e"\]\] "<div>abc{<b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["delete",""\],["inserttext","d"\],["inserttext","e"\]\] "<div>abc[<s></s><b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
@ -1257,9 +1224,3 @@
[[["forwarddelete",""\],["inserttext","d"\],["inserttext","e"\]\] "<div>abc{<s></s><b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["insertlinebreak",""\],["inserttext","d"\],["inserttext","e"\]\] "<div>abc[<s></s><b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL
[[["insertlinebreak",""\],["inserttext","d"\],["inserttext","e"\]\] "<div>abc{<s></s><b><i>de\]f</i></b>ghi</div>" compare innerHTML]
expected: FAIL

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

@ -2481,27 +2481,20 @@ var browserTests = [
[["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>"],
"<div><b><i>abc</i></b><i>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>"],
"<div><b><i>abc</i></b><i>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>"],
"<div><b><i>abc</i></b><i>d<br></i><b><br></b></div>",
[true,true,true],
{"bold":[false,true,"",false,false,""]}],
// Tests putting caret to right paragraph at insert paragraph, and preserve