From 59e1bdff221f4a099f6000eb9884f84f772c38bb Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Wed, 17 Jan 2018 14:31:35 +0100 Subject: [FIX] web_editor, website: properly clean snippets before saving /!\ Partial backport of https://github.com/odoo/odoo/commit/0a31cb9b4ae461119ecafdd05c5b979621ffb039 /!\ Should not be forward-ported Before the websitepocalypse introduced in 11.0 (saas-17), when saving a page in the editor, every `cleanForSave` method of snippet options were called, even for snippet options which were not initialized (so if a snippet was left unchanged before saving, the related `cleanForSave` methods were called anyway). This behavior was bad code and did not make much sense and was so removed with the websitepocalypse (see https://github.com/odoo/odoo/commit/2972976962617d4b8a0113bae58c640ab41cdff8#diff-4c580abda3220e45c6b3f6bdcf77c733L415) Unfortunatly, the behavior was necessary given the states of our snippet options. Indeed, for the "latest posts" snippet, the `cleanForSave` method removes the entire snippet content (as it is dynamically loaded). The dynamic loading is performed by the snippet *animation*... and removing the dynamic content should logically be done by the animation too... which is the case. The problem is that animations are not stopped before the editor is saved. This behavior was only implemented in saas-11.1 (for future version 12.0) with commit https://github.com/odoo/odoo/commit/0a31cb9b4ae461119ecafdd05c5b979621ffb039 As a stable fix for 11.0, part of the mentioned commit is backported here and some specific `cleanForSave` will be reviewed as animations if necessary. Note: `cleanForSave` made useless by this commit are left anyway as this is a stable fix. See https://github.com/odoo/odoo/issues/22089 --- addons/web_editor/static/src/js/editor/snippets.editor.js | 1 + 1 file changed, 1 insertion(+) (limited to 'addons/web_editor/static/src') diff --git a/addons/web_editor/static/src/js/editor/snippets.editor.js b/addons/web_editor/static/src/js/editor/snippets.editor.js index 5f721fd4805..1c208956624 100644 --- a/addons/web_editor/static/src/js/editor/snippets.editor.js +++ b/addons/web_editor/static/src/js/editor/snippets.editor.js @@ -622,6 +622,7 @@ var SnippetsMenu = Widget.extend({ * - Remove the 'contentEditable' attributes */ cleanForSave: function () { + this.trigger_up('ready_to_clean_for_save'); _.each(this.snippetEditors, function (snippetEditor) { snippetEditor.cleanForSave(); }); -- cgit v1.2.3 From cdb2cf744f4c9be7d2d498e28563a36aff1e0ae3 Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Thu, 18 Jan 2018 14:26:53 +0100 Subject: [FIX] web_editor: DOMElement used as jQuery element The function receives a jQuery element most of the time but it may receive a DOMElement in some cases. --- addons/web_editor/static/src/js/rte.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'addons/web_editor/static/src') diff --git a/addons/web_editor/static/src/js/rte.js b/addons/web_editor/static/src/js/rte.js index c3a88007542..20c70e9d235 100644 --- a/addons/web_editor/static/src/js/rte.js +++ b/addons/web_editor/static/src/js/rte.js @@ -271,10 +271,11 @@ var RTE = Widget.extend({ * @param {DOM} target where the dom is changed is editable zone */ historyRecordUndo: function ($target, event, internal_history) { + $target = $($target); var rng = range.create(); var $editable = $(rng && rng.sc).closest(".o_editable"); if (!rng || !$editable.length) { - $editable = $($target).closest(".o_editable"); + $editable = $target.closest(".o_editable"); rng = range.create($target.closest("*")[0],0); } else { rng = $editable.data('range') || rng; -- cgit v1.2.3 From f314cf0bb3dd715773e5ab1a5fcacd42bd3bd7fd Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Wed, 17 Jan 2018 18:45:35 +0100 Subject: [FIX] web_editor: `deleteContents` when no selection Changing the `deleteContents` method of the editor is tricky. This commit however dares to add a behavior: if the range does not represent a selection but only the cursor position and is then asked to delete its content... it should not do anything as nothing is selected. --- addons/web_editor/static/src/js/summernote.js | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'addons/web_editor/static/src') diff --git a/addons/web_editor/static/src/js/summernote.js b/addons/web_editor/static/src/js/summernote.js index 8385a79fb39..9760ef8c2a2 100644 --- a/addons/web_editor/static/src/js/summernote.js +++ b/addons/web_editor/static/src/js/summernote.js @@ -917,6 +917,10 @@ range.WrappedRange.prototype.isOnImg = function () { return nb === 1 && image; }; range.WrappedRange.prototype.deleteContents = function (towrite) { + if (this.sc === this.ec && this.so === this.eo) { + return this; + } + var prevBP = dom.removeBetween(this.sc, this.so, this.ec, this.eo, towrite); $(dom.node(prevBP.sc)).trigger("click"); // trigger click to disable and reanable editor and image handler -- cgit v1.2.3 From 216edc206c73cbde193e178c91d71d565c2097dc Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Fri, 19 Jan 2018 14:02:24 +0100 Subject: [FIX] web_editor: properly evaluate if a range is an image The previous implementation apparently came from an old version of summernote (see https://github.com/odoo/odoo/commit/d33086c03fc1a0882be38ae62989b7f72df20d3b). This commits should improve it to handle all known cases. See https://github.com/odoo/odoo/issues/20877 --- addons/web_editor/static/src/js/summernote.js | 62 +++++++++++++++++++-------- 1 file changed, 44 insertions(+), 18 deletions(-) (limited to 'addons/web_editor/static/src') diff --git a/addons/web_editor/static/src/js/summernote.js b/addons/web_editor/static/src/js/summernote.js index 9760ef8c2a2..1f2176f3751 100644 --- a/addons/web_editor/static/src/js/summernote.js +++ b/addons/web_editor/static/src/js/summernote.js @@ -886,34 +886,60 @@ range.WrappedRange.prototype.reRange = function (keep_end, isNotBreakable) { return new range.WrappedRange(sc, so, ec, eo); }; -// isOnImg: judge whether range is an image node or not +/** + * Returns the image the range is in or matches (if any, false otherwise). + * + * @todo this implementation may not cover all corner cases but should do the + * trick for all reproductible ones + * @returns {DOMElement|boolean} + */ range.WrappedRange.prototype.isOnImg = function () { + // If not a selection but a cursor position, just check if a point's + // ancestor is an image or not + if (this.sc === this.ec && this.so === this.eo) { + return dom.ancestor(this.sc, dom.isImg); + } + + var startPoint = {node: this.sc, offset: this.so}; + var endPoint = {node: this.ec, offset: this.eo}; + var nb = 0; var image; - var startPoint = {node: this.sc.childNodes.length && this.sc.childNodes[this.so] || this.sc}; - startPoint.offset = startPoint.node === this.sc ? this.so : 0; - var endPoint = {node: this.ec.childNodes.length && this.ec.childNodes[this.eo] || this.ec}; - endPoint.offset = endPoint.node === this.ec ? this.eo : 0; - - if (dom.isImg(startPoint.node)) { - nb ++; - image = startPoint.node; - } + var textNode; dom.walkPoint(startPoint, endPoint, function (point) { - if (!dom.isText(endPoint.node) && point.node === endPoint.node && point.offset === endPoint.offset) { + // If the element has children (not a text node and not empty node), + // the element cannot be considered as selected (these children will + // be processed to determine that) + if (dom.hasChildren(point.node)) { return; } - var node = point.node.childNodes.length && point.node.childNodes[point.offset] || point.node; - var offset = node === point.node ? point.offset : 0; - var isImg = dom.ancestor(node, dom.isImg); - if (!isImg && ((!dom.isBR(node) && !dom.isText(node)) || (offset && node.textContent.length !== offset && node.textContent.match(/\S|\u00A0/)))) { + + // Check if an ancestor of the current point is an image + var pointImg = dom.ancestor(point.node, dom.isImg); + var isText = dom.isText(point.node); + + // Check if a visible element is selected, i.e. + // - If an ancestor of the current is an image we did not see yet + // - If the point is not in a br or a text (so a node with no children) + // - If the point is in a non empty text node we already saw + if (pointImg ? + (image !== pointImg) : + ((!dom.isBR(point.node) && !isText) || (textNode === point.node && point.node.textContent.match(/\S|\u00A0/)))) { nb++; } - if (isImg && image !== isImg) { - image = isImg; - nb ++; + + // If an ancestor of the current point is an image, then save it as the + // image we are looking for + if (pointImg) { + image = pointImg; + } + // If the current point is a text node save it as the last text node + // seen (if we see it again, this might mean it is selected) + if (isText) { + textNode = point.node; } }); + return nb === 1 && image; }; range.WrappedRange.prototype.deleteContents = function (towrite) { -- cgit v1.2.3