Bug 1297414 - formatBlock should move non-editable nodes; r=masayuki

Before this change, if you did formatBlock on "a<span
contenteditable=false>b</span>c", it would become "acb", because the "a"
and "c" would be moved to the front of the node, and the "b" would be
left alone at the end because the editing code doesn't want to move it.
Now we will move the "b" as well, even though it's not editable, so that
the node remains "abc".  The rule is that a non-editable element cannot
have its attributes or children changed, but it can have its parent
changed, so there's nothing wrong with moving it here.

On the way, I fixed an exception in insert*List if there was an
uneditable inline node around.  I don't intend to fix all the todo's in
the test, but now it should have better coverage, at least.

MozReview-Commit-ID: 3okcGq4an3f
This commit is contained in:
Aryeh Gregor 2016-08-24 20:54:03 +03:00
Родитель e3c9b3f2de
Коммит a7ed0e7cf4
6 изменённых файлов: 136 добавлений и 77 удалений

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

@ -262,7 +262,6 @@ skip-if = toolkit == 'android' #TIMED_OUT
[test_bug424698.html]
[test_bug428135.xhtml]
[test_bug430351.html]
[test_bug430392.html]
[test_bug441930.html]
[test_bug442801.html]
[test_bug448166.html]

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

@ -1,47 +0,0 @@
<!DOCTYPE HTML>
<html>
<!--
https://bugzilla.mozilla.org/show_bug.cgi?id=430392
-->
<head>
<title>Test for Bug 430392</title>
<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
<script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
</head>
<body>
<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=430392">Mozilla Bug 430392</a>
<p id="display"></p>
<div id="content">
<div contenteditable="true" id="edit"> <span contenteditable="false">A</span> ; <span contenteditable="false">B</span> ; <span contenteditable="false">C</span> </div>
</div>
<pre id="test">
<script class="testbody" type="text/javascript">
/** Test for Bug 430392 **/
function test() {
var edit = document.getElementById("edit");
var html = edit.innerHTML;
document.getElementById("edit").focus();
synthesizeKey("VK_RIGHT", {});
synthesizeKey("VK_RIGHT", {});
synthesizeKey("VK_RETURN", {});
synthesizeKey("VK_RETURN", {});
synthesizeKey("VK_BACK_SPACE", {});
synthesizeKey("VK_BACK_SPACE", {});
is(edit.innerHTML, html,
"adding and then deleting returns should not change text");
SimpleTest.finish();
}
SimpleTest.waitForExplicitFinish();
addLoadEvent(test);
</script>
</pre>
</body>
</html>

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

@ -3229,18 +3229,18 @@ HTMLEditRules::WillMakeList(Selection* aSelection,
curList = nullptr;
}
// if curNode is a Break, delete it, and quit remembering prev list item
if (TextEditUtils::IsBreak(curNode)) {
NS_ENSURE_STATE(mHTMLEditor);
rv = mHTMLEditor->DeleteNode(curNode);
NS_ENSURE_SUCCESS(rv, rv);
prevListItem = nullptr;
continue;
} else if (IsEmptyInline(curNode)) {
// if curNode is an empty inline container, delete it
// If curNode is a break, delete it, and quit remembering prev list item.
// If an empty inline container, delete it, but still remember the previous
// item.
NS_ENSURE_STATE(mHTMLEditor);
if (mHTMLEditor->IsEditable(curNode) && (TextEditUtils::IsBreak(curNode) ||
IsEmptyInline(curNode))) {
NS_ENSURE_STATE(mHTMLEditor);
rv = mHTMLEditor->DeleteNode(curNode);
NS_ENSURE_SUCCESS(rv, rv);
if (TextEditUtils::IsBreak(curNode)) {
prevListItem = nullptr;
}
continue;
}
@ -3505,13 +3505,6 @@ HTMLEditRules::WillMakeBasicBlock(Selection& aSelection,
arrayOfNodes);
NS_ENSURE_SUCCESS(rv, rv);
// Remove all non-editable nodes. Leave them be.
for (int32_t i = arrayOfNodes.Length() - 1; i >= 0; i--) {
if (!htmlEditor->IsEditable(arrayOfNodes[i])) {
arrayOfNodes.RemoveElementAt(i);
}
}
// If nothing visible in list, make an empty block
if (ListIsEmptyLine(arrayOfNodes)) {
// Get selection location
@ -6781,6 +6774,9 @@ HTMLEditRules::RemoveBlockStyle(nsTArray<OwningNonNull<nsINode>>& aNodeArray)
NS_ENSURE_SUCCESS(rv, rv);
firstNode = lastNode = curBlock = nullptr;
}
if (!mHTMLEditor->IsEditable(curNode)) {
continue;
}
// Remove current block
nsresult rv = htmlEditor->RemoveBlockContainer(*curNode->AsContent());
NS_ENSURE_SUCCESS(rv, rv);
@ -6798,6 +6794,9 @@ HTMLEditRules::RemoveBlockStyle(nsTArray<OwningNonNull<nsINode>>& aNodeArray)
NS_ENSURE_SUCCESS(rv, rv);
firstNode = lastNode = curBlock = nullptr;
}
if (!mHTMLEditor->IsEditable(curNode)) {
continue;
}
// Recursion time
nsTArray<OwningNonNull<nsINode>> childArray;
GetChildNodesForOperation(*curNode, childArray);
@ -6820,11 +6819,12 @@ HTMLEditRules::RemoveBlockStyle(nsTArray<OwningNonNull<nsINode>>& aNodeArray)
// Fall out and handle curNode
}
curBlock = htmlEditor->GetBlockNodeParent(curNode);
if (curBlock && HTMLEditUtils::IsFormatNode(curBlock)) {
firstNode = lastNode = curNode->AsContent();
} else {
if (!curBlock || !HTMLEditUtils::IsFormatNode(curBlock) ||
!mHTMLEditor->IsEditable(curBlock)) {
// Not a block kind that we care about.
curBlock = nullptr;
} else {
firstNode = lastNode = curNode->AsContent();
}
} else if (curBlock) {
// Some node that is already sans block style. Skip over it and process
@ -6857,13 +6857,6 @@ HTMLEditRules::ApplyBlockStyle(nsTArray<OwningNonNull<nsINode>>& aNodeArray,
NS_ENSURE_STATE(mHTMLEditor);
RefPtr<HTMLEditor> htmlEditor(mHTMLEditor);
// Remove all non-editable nodes. Leave them be.
for (int32_t i = aNodeArray.Length() - 1; i >= 0; i--) {
if (!htmlEditor->IsEditable(aNodeArray[i])) {
aNodeArray.RemoveElementAt(i);
}
}
nsCOMPtr<Element> newBlock;
nsCOMPtr<Element> curBlock;
@ -6871,8 +6864,9 @@ HTMLEditRules::ApplyBlockStyle(nsTArray<OwningNonNull<nsINode>>& aNodeArray,
nsCOMPtr<nsINode> curParent = curNode->GetParentNode();
int32_t offset = curParent ? curParent->IndexOf(curNode) : -1;
// Is it already the right kind of block?
if (curNode->IsHTMLElement(&aBlockTag)) {
// Is it already the right kind of block, or an uneditable block?
if (curNode->IsHTMLElement(&aBlockTag) ||
(!mHTMLEditor->IsEditable(curNode) && IsBlockNode(curNode))) {
// Forget any previous block used for previous inline nodes
curBlock = nullptr;
// Do nothing to this block

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

@ -3168,7 +3168,7 @@ HTMLEditor::DeleteNode(nsIDOMNode* aNode)
{
// do nothing if the node is read-only
nsCOMPtr<nsIContent> content = do_QueryInterface(aNode);
if (!IsModifiableNode(aNode) && !IsMozEditorBogusNode(content)) {
if (NS_WARN_IF(!IsModifiableNode(aNode) && !IsMozEditorBogusNode(content))) {
return NS_ERROR_FAILURE;
}

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

@ -37,6 +37,7 @@ skip-if = toolkit == 'android'
[test_bug414526.html]
[test_bug417418.html]
skip-if = android_version == '18' # bug 1147989
[test_bug430392.html]
[test_bug432225.html]
skip-if = toolkit == 'android'
[test_bug439808.html]

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

@ -0,0 +1,112 @@
<!DOCTYPE HTML>
<html>
<!--
https://bugzilla.mozilla.org/show_bug.cgi?id=430392
-->
<head>
<title>Test for Bug 430392</title>
<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
<script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
</head>
<body>
<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=430392">Mozilla Bug 430392</a>
<p id="display"></p>
<div id="content">
<div contenteditable="true" id="edit"> <span contenteditable="false">A</span> ; <span contenteditable="false">B</span> ; <span contenteditable="false">C</span> </div>
</div>
<pre id="test">
<script class="testbody" type="text/javascript">
/** Test for Bug 430392 **/
function test() {
var edit = document.getElementById("edit");
var html = edit.innerHTML;
var expectedText = edit.textContent;
document.getElementById("edit").focus();
// Each test is [desc, callback]. callback() is called and we check that the
// textContent didn't change. For expected failures, the format is [desc,
// callback, expectedValue], and the test will be marked as an expected fail
// if the textContent changes to expectedValue, and an unexpected fail if
// it's neither the original value nor expectedValue.
var tests = [["adding returns", () => {
getSelection().collapse(edit.firstChild, 0);
synthesizeKey("VK_RIGHT", {});
synthesizeKey("VK_RIGHT", {});
synthesizeKey("VK_RETURN", {});
synthesizeKey("VK_RETURN", {});
synthesizeKey("VK_BACK_SPACE", {});
synthesizeKey("VK_BACK_SPACE", {});
}], ["adding shift-returns", () => {
getSelection().collapse(edit.firstChild, 0);
synthesizeKey("VK_RIGHT", {});
synthesizeKey("VK_RIGHT", {});
synthesizeKey("VK_RETURN", {shiftKey: true});
synthesizeKey("VK_RETURN", {shiftKey: true});
synthesizeKey("VK_BACK_SPACE", {});
synthesizeKey("VK_BACK_SPACE", {});
}, "A ; B ; C "]];
[
"insertorderedlist",
"insertunorderedlist",
["formatblock", "p"],
]
.forEach(item => {
var cmd = Array.isArray(item) ? item[0] : item;
var param = Array.isArray(item) ? item[1] : "";
tests.push([cmd, () => { document.execCommand(cmd, false, param) }]);
});
// These are all TODO -- they don't move the non-editable elements
[
"bold",
"italic",
"underline",
"strikethrough",
"subscript",
"superscript",
["forecolor", "blue"],
["backcolor", "blue"],
["hilitecolor", "blue"],
["fontname", "monospace"],
["fontsize", "1"],
"justifyright",
"justifycenter",
"justifyfull"
]
.forEach(item => {
var cmd = Array.isArray(item) ? item[0] : item;
var param = Array.isArray(item) ? item[1] : "";
tests.push([cmd, () => { document.execCommand(cmd, false, param) },
" A ; ; BC "]);
});
tests.push(["indent", () => { document.execCommand("indent") },
" ; ; ABC"]);
tests.forEach(arr => {
edit.innerHTML = html;
edit.focus();
getSelection().selectAllChildren(edit);
arr[1]();
if (2 < arr.length) {
todo_is(edit.textContent, expectedText,
arr[0] + " should not change text");
if (edit.textContent !== expectedText && edit.textContent !== arr[2]) {
is(edit.textContent, arr[2],
arr[0] + " changed to different failure");
}
} else {
is(edit.textContent, expectedText, arr[0] + " should not change text");
}
});
SimpleTest.finish();
}
SimpleTest.waitForExplicitFinish();
SimpleTest.waitForFocus(test);
</script>
</pre>
</body>
</html>