From 41088bb311c8db92242c4c2479f307fb6e5731c4 Mon Sep 17 00:00:00 2001
From: Antoine Guenet <age@odoo.com>
Date: Tue, 16 Apr 2019 08:10:14 +0000
Subject: [PATCH] [FIX] web_editor: properly handle delete in nested lists of
all syntaxes
This addresses various bugs that occured with nested lists - including
lists that are nested with a UL within a LI (as opposed to a UL directly
within another UL).
---
.../static/src/js/wysiwyg/plugin/helper.js | 55 +++++++++++----
.../static/tests/wysiwyg_keyboard_tests.js | 12 ++++
.../web_editor/static/tests/wysiwyg_tests.js | 67 ++++++++++++++++++-
3 files changed, 118 insertions(+), 16 deletions(-)
diff --git a/addons/web_editor/static/src/js/wysiwyg/plugin/helper.js b/addons/web_editor/static/src/js/wysiwyg/plugin/helper.js
index 4ba44fe3c9b3..af903dd212e5 100644
--- a/addons/web_editor/static/src/js/wysiwyg/plugin/helper.js
+++ b/addons/web_editor/static/src/js/wysiwyg/plugin/helper.js
@@ -109,12 +109,14 @@ var HelperPlugin = AbstractPlugin.extend({
this.orderStyle(node);
this.orderClass(otherNode);
this.orderStyle(otherNode);
- if (node.attributes.length !== otherNode.attributes.length) {
+ var nodeAttributes = this._getAttrsNoID(node);
+ var otherNodeAttributes = this._getAttrsNoID(otherNode);
+ if (nodeAttributes.length !== otherNodeAttributes.length) {
return false;
}
- for (var i = 0; i < node.attributes.length; i++) {
- var attr = node.attributes[i];
- var otherAttr = otherNode.attributes[i];
+ for (var i = 0; i < nodeAttributes.length; i++) {
+ var attr = nodeAttributes[i];
+ var otherAttr = otherNodeAttributes[i];
if (attr.name !== otherAttr.name || attr.value !== otherAttr.value) {
return false;
}
@@ -455,6 +457,11 @@ var HelperPlugin = AbstractPlugin.extend({
return this.makePoint(this.firstLeaf(blockToMergeFrom), 0);
}
+ // UL/OL can only have LI, UL or OR as direct children
+ if (dom.isList(blockToMergeInto) && !dom.isLi(blockToMergeFrom) && !dom.isList(blockToMergeFrom)) {
+ return;
+ }
+
return this.mergeNonSimilarBlocks(blockToMergeFrom, blockToMergeInto);
},
/**
@@ -601,22 +608,24 @@ var HelperPlugin = AbstractPlugin.extend({
var li = dom.ancestor(node, function (n) {
return n !== node && self.isNodeBlockType(n) || dom.isLi(n);
});
+ var nextElementSibling = direction === 'next' ? 'nextElementSibling' : 'previousElementSibling';
li = li && dom.isLi(li) ? li : undefined;
- if (li && direction === 'next') {
- if (li.nextElementSibling) {
+ if (li) {
+ if (li[nextElementSibling]) {
node = li;
} else {
node = dom.ancestor(node, function (n) {
- return ((n.tagName === 'UL' || n.tagName === 'OL') && n.nextElementSibling);
+ return ((n.tagName === 'UL' || n.tagName === 'OL') && n[nextElementSibling] &&
+ (direction === 'next' || dom.isList(n[nextElementSibling])));
});
}
}
- if (!node || !node[direction === 'next' ? 'nextElementSibling' : 'previousElementSibling']) {
+ if (!node || !node[nextElementSibling]) {
return false;
}
- node = node[direction === 'next' ? 'nextElementSibling' : 'previousElementSibling'];
+ node = node[nextElementSibling];
var ulFoldedSnippetNode = dom.ancestor(node, function (n) {
return $(n).hasClass('o_ul_folded');
@@ -633,7 +642,7 @@ var HelperPlugin = AbstractPlugin.extend({
node = this.firstBlockAncestor(node);
- li = dom.ancestor(node, function (n) {
+ li = dom.isList(node) ? undefined : dom.ancestor(node, function (n) {
return n !== node && self.isNodeBlockType(n) || dom.isLi(n);
});
li = li && dom.isLi(li) ? li : undefined;
@@ -1051,9 +1060,15 @@ var HelperPlugin = AbstractPlugin.extend({
* but without any.
*
* @param {Node} node
+ * @param {Boolean} brIsBlank true to consider nodes with BR element
+ * as ultimate only child to be blank
* @returns {Boolean}
*/
- isBlankNode: function (node) {
+ isBlankNode: function (node, brIsBlank) {
+ var self = this;
+ if (brIsBlank && dom.isBR(node)) {
+ return true;
+ }
if (dom.isVoid(node) || dom.isIcon(node)) {
return false;
}
@@ -1062,7 +1077,10 @@ var HelperPlugin = AbstractPlugin.extend({
}).test(node[dom.isText(node) ? 'textContent' : 'innerHTML'])) {
return true;
}
- if (node.childNodes.length && _.all(node.childNodes, this.isBlankNode.bind(this))) {
+ var areAllChildrenBlank = _.all(node.childNodes, function (node) {
+ return self.isBlankNode(node, brIsBlank);
+ });
+ if (node.childNodes.length && areAllChildrenBlank) {
return true;
}
return false;
@@ -1306,7 +1324,7 @@ var HelperPlugin = AbstractPlugin.extend({
$lastContents.after($contents);
}
}
- while (mergeFromBlock.parentNode && this.isBlankNode(mergeFromBlock.parentNode)) {
+ while (mergeFromBlock.parentNode && this.isBlankNode(mergeFromBlock.parentNode, true)) {
mergeFromBlock = mergeFromBlock.parentNode;
}
$(mergeFromBlock).remove();
@@ -1478,7 +1496,7 @@ var HelperPlugin = AbstractPlugin.extend({
if (isAfterBlank) {
$(node.previousSibling).remove();
}
- var isBeforeBlank = node.nextSibling && this.isBlankNode(node.nextSibling)
+ var isBeforeBlank = node.nextSibling && this.isBlankNode(node.nextSibling);
if (isBeforeBlank) {
$(node.nextSibling).remove();
}
@@ -1733,6 +1751,15 @@ var HelperPlugin = AbstractPlugin.extend({
// Private
//--------------------------------------------------------------------------
+ _getAttrsNoID: function (node) {
+ var nodeAttributes = [];
+ for (var i = 0; i < node.attributes.length; i++) {
+ if (node.attributes[i].name !== 'id') {
+ nodeAttributes.push(node.attributes[i]);
+ }
+ }
+ return nodeAttributes;
+ },
_insertTextNodeInEditableArea: function (range, text) {
// try to insert the text node in editable area
var textNode = this.document.createTextNode(text);
diff --git a/addons/web_editor/static/tests/wysiwyg_keyboard_tests.js b/addons/web_editor/static/tests/wysiwyg_keyboard_tests.js
index 28a8106234c6..14505ebe77fc 100644
--- a/addons/web_editor/static/tests/wysiwyg_keyboard_tests.js
+++ b/addons/web_editor/static/tests/wysiwyg_keyboard_tests.js
@@ -2103,6 +2103,18 @@ var keyboardTestsComplex = [{
start: "h1:contents()[0]->15",
},
},
+ {
+ name: "in ul > li > ul > 1st li > empty p (other ul li ul before): BACKSPACE at start",
+ content: '<ul><li><p>1</p></li><li class="o_indent"><ul><li><p>2</p></li></ul><ul class="o_checklist"><li id="checklist-id-1"><p><br></p></li></ul></li><li><p>3</p></li></ul>',
+ steps: [{
+ start: "p:eq(2)->0",
+ key: 'BACKSPACE',
+ }],
+ test: {
+ content: '<ul><li><p>1</p></li><li class="o_indent"><ul><li><p>2</p></li></ul></li><li><p>3</p></li></ul>',
+ start: "p:eq(1):contents()[0]->1",
+ },
+ },
];
QUnit.test('Complex', function (assert) {
diff --git a/addons/web_editor/static/tests/wysiwyg_tests.js b/addons/web_editor/static/tests/wysiwyg_tests.js
index 98c08f504b89..7b0b5899cef9 100644
--- a/addons/web_editor/static/tests/wysiwyg_tests.js
+++ b/addons/web_editor/static/tests/wysiwyg_tests.js
@@ -2498,16 +2498,20 @@ QUnit.test('Indent/outdent', function (assert) {
QUnit.test('checklist', function (assert) {
var done = assert.async();
- assert.expect(10);
+ assert.expect(11);
return weTestUtils.createWysiwyg({
debug: false,
wysiwygOptions: {
-
+ generateOptions: function (options) {
+ options.toolbar[4][1] = ['ul', 'ol', 'checklist', 'paragraph'];
+ },
},
}).then(function (wysiwyg) {
var $editable = wysiwyg.$('.note-editable');
+ var $btnChecklist = wysiwyg.$('.note-para .fa-check-square');
+
var checklistTests = [
{
name: "check checkbox in checklist with children",
@@ -2869,11 +2873,70 @@ QUnit.test('checklist', function (assert) {
'<p>y</p>',
},
},
+ {
+ name: "convert 2 ul li ul li into two ul li ul.o_checklist li",
+ content: '<ul>' +
+ '<li>' +
+ '<p>1</p>' +
+ '</li>' +
+ '<li class="o_indent">' +
+ '<ul>' +
+ '<li>' +
+ '<p>2</p>' +
+ '</li>' +
+ '<li>' +
+ '<p>3</p>' +
+ '</li>' +
+ '<li>' +
+ '<p>4</p>' +
+ '</li>' +
+ '</ul>' +
+ '</li>' +
+ '<li>' +
+ '<p>5</p>' +
+ '</li>' +
+ '</ul>',
+ start: 'p:eq(2):contents()[0]->0',
+ end: 'p:eq(3):contents()[0]->1',
+ do: function () {
+ $btnChecklist.mousedown().click();
+ },
+ test: {
+ content: '<ul>' +
+ '<li>' +
+ '<p>1</p>' +
+ '</li>' +
+ '<li class="o_indent">' +
+ '<ul>' +
+ '<li>' +
+ '<p>2</p>' +
+ '</li>' +
+ '</ul>' +
+ '<ul class="o_checklist">' +
+ '<li>' +
+ '<p>3</p>' +
+ '</li>' +
+ '<li>' +
+ '<p>4</p>' +
+ '</li>' +
+ '</ul>' +
+ '</li>' +
+ '<li>' +
+ '<p>5</p>' +
+ '</li>' +
+ '</ul>',
+ },
+ },
];
_.each(checklistTests, function (test) {
testName = test.name;
wysiwyg.setValue(test.content);
+ if (test.start) {
+ var range = weTestUtils.select(test.start, test.end || test.start, $editable);
+ $(range.sc).mousedown();
+ Wysiwyg.setRange(range.sc, range.so, range.ec, range.eo);
+ }
test.do();
assert.deepEqual(wysiwyg.getValue(), test.test.content, testName);
});
--
GitLab