Skip to content

Commit 31ba956

Browse files
committed
[FIX] composer: fix get/setText of the content editable helper
The recent use of the property "plaintext-only" came with a change of behaviour in the composers, specifically in Firefox. How to reproduce in FF: - write a single character in a grid composer - delete the character -> a new line is inserted Apparently, when setting the attribute `contentEditable` to `plaintext-only`, deleting the single character does not delete the span that encapsulates it if the latter has a class attribute. This couples with a recent refactoring of the content editable helper to handle new line characters and the empty composer is not detected as such What happens currently? Consider the composer content after inputing the letter 'W': ```html <p> <span class=""> w </span> </p> ``` The cursor is set just after the letter W. In a chromium-based browser, hitting the `Delete` key yeilds the following result: ```html <p> <span> <br> </span> </p> ``` The letter is replaced by `<br>` which could be interpreted as a newline, except that in this situation, we do not want a newline to appear as we meerly deleted the single character of the composer, so we do not want new characters. Also, we introduced some logic to fight this side-effect when we introduced the support of newlines of pasted content (#6467) However, it turns out that in firefox, hitting `Delete` will yield this result ```html <p> <span class=""> <br> </span> </p> ``` The class attribute is not cleared up! And unfortunately, the logic that counteracts the presence of the unwelcome `<br>` would test against the exact form seen in a chromium-based browser, so it did not work on Firefox. This commit extends the check to work on both Firefox-based and Chromium-based browsers. Note that other leads were investigated and it seems that if we stopped using paragraphs to represent new lines, this issue with the `<br>` doesn't seem to occur. An improvement (and good cleanup) could be achieved by dropping that paragraph strategy and rely on spans and br characters. closes #7394 Task: 5082601 X-original-commit: 828ee0b Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Rémi Rahir (rar) <[email protected]>
1 parent cae1ff8 commit 31ba956

File tree

2 files changed

+24
-3
lines changed

2 files changed

+24
-3
lines changed

src/components/composer/content_editable_helper.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,9 +249,7 @@ export class ContentEditableHelper {
249249
} else {
250250
text += NEWLINE;
251251
}
252-
emptyParagraph = ["<br>", "<span><br></span>"].includes(
253-
(current.value as HTMLElement).innerHTML
254-
);
252+
emptyParagraph = isEmptyParagraph(current.value);
255253
continue;
256254
}
257255
if (!current.value.hasChildNodes()) {
@@ -274,3 +272,17 @@ function compareContentToSpanElement(content: HtmlContent, node: HTMLElement): b
274272
const sameContent = node.innerText === content.value;
275273
return sameColor && sameClass && sameContent;
276274
}
275+
276+
const doc = new DOMParser();
277+
const brNode = doc.parseFromString("<br>", "text/html").body.firstChild;
278+
const spanBrNode = doc.parseFromString("<span><br></span>", "text/html").body.firstChild;
279+
280+
function isEmptyParagraph(node: Node) {
281+
if (node.childNodes.length > 1) return false;
282+
const node2 = node.firstChild?.cloneNode(true);
283+
if (!node2) return true;
284+
if (!(node2 instanceof Element)) return false;
285+
node2.removeAttribute("class");
286+
node2.removeAttribute("style");
287+
return node2.isEqualNode(brNode) || node2.isEqualNode(spanBrNode) || false;
288+
}

tests/composer/content_editable_helpers.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,15 @@ describe("ContentEditableHelper", () => {
133133

134134
div.innerHTML = "hello<br>test<br>";
135135
expect(helper.getText()).toBe("hello\ntest\n");
136+
137+
div.innerHTML = "<br><br><br>";
138+
expect(helper.getText()).toBe("\n\n\n");
139+
140+
div.innerHTML = "<br>";
141+
expect(helper.getText()).toBe("\n");
142+
143+
div.innerHTML = "<span><br></span>";
144+
expect(helper.getText()).toBe("\n");
136145
});
137146

138147
test("Paste multiline <p>", () => {

0 commit comments

Comments
 (0)