Skip to content

Commit

Permalink
Bug 1868664 - Make EditorBase::SetTextDirectionTo use active editin…
Browse files Browse the repository at this point in the history
…g host if it's an `HTMLEditor` r=m_kato

Chrome handles text direction change as a block level format command.  I.e.,
Chrome sets `dir` attributes of enclosing editable blocks of selected leaf
nodes.  However, aligning to this may require a lot of change and Thunderbird
may require the traditional behavior.

On the other hand, it's obviously wrong thing that `Ctrl` + `Shift` + `X` in an
editable element changes the direction of entire the document since the `<body>`
may not be editable.

Therefore, this patch just changes the direction change target from `<body>`
to active editing host if it's an `HTMLEditor`.

Differential Revision: https://phabricator.services.mozilla.com/D195838
  • Loading branch information
masayuki-nakano committed Dec 11, 2023
1 parent 759c68c commit 78cd50d
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 14 deletions.
17 changes: 12 additions & 5 deletions editor/libeditor/EditorBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5664,14 +5664,21 @@ void EditorBase::SwitchTextDirectionTo(TextDirection aTextDirection) {
}

nsresult EditorBase::SetTextDirectionTo(TextDirection aTextDirection) {
Element* rootElement = GetExposedRoot();
Element* const editingHostOrTextControlElement =
IsHTMLEditor() ? AsHTMLEditor()->ComputeEditingHost(
HTMLEditor::LimitInBodyElement::No)
: GetExposedRoot();
if (!editingHostOrTextControlElement) { // Don't warn, HTMLEditor may have no
// active editing host
return NS_OK;
}

if (aTextDirection == TextDirection::eLTR) {
NS_ASSERTION(!IsLeftToRight(), "Unexpected mutually exclusive flag");
mFlags &= ~nsIEditor::eEditorRightToLeft;
mFlags |= nsIEditor::eEditorLeftToRight;
nsresult rv = rootElement->SetAttr(kNameSpaceID_None, nsGkAtoms::dir,
u"ltr"_ns, true);
nsresult rv = editingHostOrTextControlElement->SetAttr(
kNameSpaceID_None, nsGkAtoms::dir, u"ltr"_ns, true);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
"Element::SetAttr(nsGkAtoms::dir, ltr) failed");
return rv;
Expand All @@ -5681,8 +5688,8 @@ nsresult EditorBase::SetTextDirectionTo(TextDirection aTextDirection) {
NS_ASSERTION(!IsRightToLeft(), "Unexpected mutually exclusive flag");
mFlags |= nsIEditor::eEditorRightToLeft;
mFlags &= ~nsIEditor::eEditorLeftToRight;
nsresult rv = rootElement->SetAttr(kNameSpaceID_None, nsGkAtoms::dir,
u"rtl"_ns, true);
nsresult rv = editingHostOrTextControlElement->SetAttr(
kNameSpaceID_None, nsGkAtoms::dir, u"rtl"_ns, true);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
"Element::SetAttr(nsGkAtoms::dir, rtl) failed");
return rv;
Expand Down
2 changes: 2 additions & 0 deletions editor/libeditor/tests/mochitest.toml
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,8 @@ skip-if = ["os == 'android'"]

["test_htmleditor_tab_key_handling.html"]

["test_htmleditor_toggle_text_direction.html"]

["test_initial_selection_and_caret_of_designMode.html"]

["test_inlineTableEditing.html"]
Expand Down
18 changes: 9 additions & 9 deletions editor/libeditor/tests/test_dom_input_event_on_htmleditor.html
Original file line number Diff line number Diff line change
Expand Up @@ -1162,8 +1162,9 @@
});

function test_switching_text_direction_from_default(aTestData) {
const editingHost = aDocument.getElementById("editTarget") || aDocument.getElementById("eventTarget");
try {
body.removeAttribute("dir");
editingHost.removeAttribute("dir");
htmlEditor.flags &= ~SpecialPowers.Ci.nsIEditor.eEditorRightToLeft;
htmlEditor.flags |= SpecialPowers.Ci.nsIEditor.eEditorLeftToRight; // XXX flags update is required, must be a bug.
aDocument.documentElement.scrollTop; // XXX Update the body frame
Expand All @@ -1173,12 +1174,11 @@
aTestData,
() => {
SpecialPowers.doCommand(aWindow, "cmd_switchTextDirection");
// XXX If editing host is a descendant of `<body>`, this must be a bug.
if (aTestData.cancelBeforeInput) {
is(body.getAttribute("dir"), null,
is(editingHost.getAttribute("dir"), null,
`${aDescription}dir attribute of the element shouldn't have been set by ${aTestData.action}`);
} else {
is(body.getAttribute("dir"), "rtl",
is(editingHost.getAttribute("dir"), "rtl",
`${aDescription}dir attribute of the element should've been set to "rtl" by ${aTestData.action}`);
}
},
Expand All @@ -1196,7 +1196,7 @@
}
);
} finally {
body.removeAttribute("dir");
editingHost.removeAttribute("dir");
htmlEditor.flags &= ~SpecialPowers.Ci.nsIEditor.eEditorRightToLeft;
htmlEditor.flags |= SpecialPowers.Ci.nsIEditor.eEditorLeftToRight; // XXX flags update is required, must be a bug.
aDocument.documentElement.scrollTop; // XXX Update the body frame
Expand All @@ -1212,8 +1212,9 @@
});

function test_switching_text_direction_from_rtl_to_ltr(aTestData) {
const editingHost = aDocument.getElementById("editTarget") || aDocument.getElementById("eventTarget");
try {
body.setAttribute("dir", "rtl");
editingHost.setAttribute("dir", "rtl");
htmlEditor.flags &= ~SpecialPowers.Ci.nsIEditor.eEditorLeftToRight;
htmlEditor.flags |= SpecialPowers.Ci.nsIEditor.eEditorRightToLeft; // XXX flags update is required, must be a bug.
aDocument.documentElement.scrollTop; // XXX Update the body frame
Expand All @@ -1223,9 +1224,8 @@
aTestData,
() => {
SpecialPowers.doCommand(aWindow, "cmd_switchTextDirection");
// XXX If editing host is a descendant of `<body>`, this must be a bug.
let expectedDirValue = aTestData.cancelBeforeInput ? "rtl" : "ltr";
is(body.getAttribute("dir"), expectedDirValue,
is(editingHost.getAttribute("dir"), expectedDirValue,
`${aDescription}dir attribute of the element should be "${expectedDirValue}" after ${aTestData.action}`);
},
{
Expand All @@ -1242,7 +1242,7 @@
}
);
} finally {
body.removeAttribute("dir");
editingHost.removeAttribute("dir");
htmlEditor.flags &= ~SpecialPowers.Ci.nsIEditor.eEditorRightToLeft;
htmlEditor.flags |= SpecialPowers.Ci.nsIEditor.eEditorLeftToRight; // XXX flags update is required, must be a bug.
aDocument.documentElement.scrollTop; // XXX Update the body frame
Expand Down
73 changes: 73 additions & 0 deletions editor/libeditor/tests/test_htmleditor_toggle_text_direction.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Text direction switch target of HTMLEditor</title>
<script src="/tests/SimpleTest/EventUtils.js"></script>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" href="/tests/SimpleTest/test.css">
<script>
"use strict";

SimpleTest.waitForExplicitFinish();
SimpleTest.waitForFocus(async () => {
await (async function test_in_contenteditable() {
document.body.innerHTML = "<div><div contenteditable>editable text</div></div>";
const editingHost = document.querySelector("div[contenteditable]");
editingHost.focus();
SpecialPowers.doCommand(window, "cmd_switchTextDirection");
is(
editingHost.getAttribute("dir"),
"rtl",
"test_in_contenteditable: dir attr of the editing host should be set"
);
is(
editingHost.parentElement.getAttribute("dir"),
null,
"test_in_contenteditable: dir attr of the parent div of the editing host should not be set"
);
is(
document.body.getAttribute("dir"),
null,
"test_in_contenteditable: dir attr of the <body> should not be set",
);
is(
document.documentElement.getAttribute("dir"),
null,
"test_in_contenteditable: dir attr of the <html> should not be set",
);
})();

await (async function test_in_designMode() {
document.body.innerHTML = "<div>abc</div>";
document.designMode = "on";
getSelection().collapse(document.querySelector("div").firstChild, 0);
SpecialPowers.doCommand(window, "cmd_switchTextDirection");
is(
document.querySelector("div").getAttribute("dir"),
null,
"test_in_designMode: dir attr of the <div> should not be set",
);
is(
document.body.getAttribute("dir"),
"rtl",
"test_in_designMode: dir attr of the <body> should be set",
);
is(
document.documentElement.getAttribute("dir"),
null,
"test_in_designMode: dir attr of the <html> should not be set",
);
document.designMode = "off";
document.body.removeAttribute("dir");
document.body.innerHTML = "";
document.documentElement.removeAttribute("dir");
})();

SimpleTest.finish();
});
</script>
</head>
<body>
</body>
</html>

0 comments on commit 78cd50d

Please sign in to comment.