Bug 1906559 - Make AutoInlineStyleSetter::OnHandled() never set point in aContent if it's not a container r=m_kato

When `HTMLEditor::InsertLinkAroundSelectionAsAction()` calls
`HTMLEditor::SetInlinePropertiesAsSubAction()`, it adds 2 elements to the array.
One is for `_moz_dirty` attribute and the other is for `href` attribute because
`InsertTagCommand::DoCommandParam()` uses
`HTMLEditor::CreateElementWithDefaults()`.

Then, `HTMLEditor::SetInlinePropertiesAsSubAction()` wraps the `<img>` into
`<a _moz_dirty="">` with calling `AutoInlineStyleSetter::ApplyStyle()`. Then,
it calls `OnHandle()` with the `<img>` and it collapse the applied range into
the `<img>`.  Therefore, when `SetInlinePropertiesAsSubAction()` handles `href`,
there is only collapsed range and it just puts the style into the cache for
applying the style when the user types new text.

So, first, `OnHandle()` should not set the boundary points of the applied range
into non-container node. And also `InsertLinkAroundSelectionAsAction()` should
ignore `_moz_dirty` attribute because if it's truly required,
`AutoInlineStyleSetter` should set the attribute to every new element.

Note that this causes new failure in `editing/run/fontname.html?1001-200`,
but it was accidentally passed.  The case is, `queryCommandValue("fontname")`
result after calling `execCommand("fontname", false, "sans-serif")` for
`foo<tt>{<br></tt>}bar`.  The result of the DOM tree is
`foo<tt><font face="sans-serif"><br></font></tt>bar` and `Selection` was
collapsed in the `<br>` before, but with this patch, the `<br>` is selected.
Therefore, the result is changed but similar cases in the tests fail too. So I
believe that it accidentally passed with odd `Selection`.

Differential Revision: https://phabricator.services.mozilla.com/D219127
This commit is contained in:
Masayuki Nakano 2024-08-19 00:35:33 +00:00
parent ceb1a86c7d
commit 66d2f119a5
4 changed files with 49 additions and 0 deletions

View File

@ -3788,6 +3788,11 @@ nsresult HTMLEditor::InsertLinkAroundSelectionAsAction(
for (const uint32_t i : IntegerRange(attrCount)) {
const BorrowedAttrInfo attrInfo = anchor->GetAttrInfoAt(i);
if (const nsAttrName* attrName = attrInfo.mName) {
// We don't need to handle _moz_dirty attribute. If it's required, the
// handler should add it to the new element.
if (attrName->IsAtom() && attrName->Equals(nsGkAtoms::mozdirty)) {
continue;
}
RefPtr<nsAtom> attributeName = attrName->LocalName();
MOZ_ASSERT(attrInfo.mValue);
nsString attrValue;

View File

@ -10,6 +10,7 @@
#include "EditorForwards.h"
#include "HTMLEditor.h" // for HTMLEditor
#include "HTMLEditHelpers.h" // for EditorInlineStyleAndValue
#include "HTMLEditUtils.h" // for HTMLEditUtils::IsContainerNode
#include "mozilla/Attributes.h"
#include "mozilla/OwningNonNull.h"
@ -186,6 +187,13 @@ class MOZ_STACK_CLASS HTMLEditor::AutoInlineStyleSetter final
mLastHandledPoint = aEndPoint;
}
void OnHandled(nsIContent& aContent) {
if (aContent.IsElement() && !HTMLEditUtils::IsContainerNode(aContent)) {
if (!mFirstHandledPoint.IsSet()) {
mFirstHandledPoint.Set(&aContent);
}
mLastHandledPoint.SetAfter(&aContent);
return;
}
if (!mFirstHandledPoint.IsSet()) {
mFirstHandledPoint.Set(&aContent, 0u);
}

View File

@ -95,6 +95,9 @@
[[["stylewithcss","false"\],["fontname","sans-serif"\]\] "foo<tt>{<br></tt>b\]ar" queryCommandValue("fontname") before]
expected: FAIL
[[["stylewithcss","false"\],["fontname","sans-serif"\]\] "foo<tt>{<br></tt>}bar" queryCommandValue("fontname") after]
expected: FAIL
[fontname.html?2001-last]
expected:

View File

@ -0,0 +1,33 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Test document.execCommand("createLink") when selecting an img element</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../include/editor-test-utils.js"></script>
<script>
"use strict";
addEventListener("load", () => {
const editingHost = document.querySelector("div[contenteditable]");
editingHost.focus();
const utils = new EditorTestUtils(editingHost);
for (const imgDisplay of ["inline", "block", "inline-block"]) {
test(t => {
utils.setupEditingHost(`{<img src="/img/lion.svg" style="display:${imgDisplay}">}`);
document.execCommand("createLink", false, "foo.html");
utils.normalizeStyleAttributeValues();
assert_equals(
editingHost.innerHTML,
`<a href="foo.html"><img src="/img/lion.svg" style="display:${imgDisplay}"></a>`
);
}, `createLink should wrap the <img style="display:${imgDisplay}>"`);
}
});
</script>
</head>
<body>
<div contenteditable></div>
</body>
</html>