From e65f7d7e309828c207ebe475b1a5830df3c45eb7 Mon Sep 17 00:00:00 2001 From: David Monjoie Date: Mon, 20 Feb 2023 14:12:47 +0000 Subject: [PATCH] [FIX] web_editor: remove pretty printing of ir.ui.view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The following basic case has been broken in website forever since [1] in Odoo 8: ```html

ab

``` Saving the above html results in: ```html

a b

``` Which, when re-rendered back in the DOM renders equivalent to: ```html

a b

``` Note the space between "a" and "b". That is because etree will pretty print nodes with indentations as long as they do not have text content, and that indentation is collapsed into a single visible space by the browser when inserted in the DOM. This is not limited to span nodes as the same applies to any inline node. This is very easily reproduced in website on any version: - Drop a Text snippet. - Replace all the content of the snippet by "ab". - Put "a" in bold and "b" in italic. - Save. - Notice that the saved version is now "a b" instead "ab". The user has no way of removing this space easily because even if they manage to do it by any mean, the server will pretty print the html again and the space will reappear. The only way to circumvent this is to have some text content as sibling of the inline nodes. Consider this: ``` >>> etree.tostring(html.fromstring('

ab

'), pretty_print=True)​ ​b'

\n a\n b\n

\n'​ ``` Which is incorrect, while this: ``` >>> etree.tostring(html.fromstring('

abc

'), pretty_print=True)​ ​b'

abc

\n'​ ``` Is correct. We could fix it using a heavy hack that would leverage this behavior by inserting one of the few unicode control characters that etree considers to be actual content, and therefore preventing pretty printing for this node. The server would then remove the control character to avoid polluting the actual views. This would have the side-effect of forbidding this control character to ever be used in a view however, and would obviously be an extremely ugly hack. The alternative which was chosen in accordance with Antony (al) and Xavier (xmo) is to disable pretty printing altogether, since the original commit [1] seem to have introduced it as a fix for an old version of the editor rather than for the intrinsic qualities of having pretty printed views. If we ever want to re-enable pretty printing in the future, I suggest it be implemented in JS because the browser is the only one able to assert whether a node is going to be treated as a block or as an inline with respect to the current CSS rules in application. task-3142796 opw-3122373 opw-3186250 [1]: https://github.com/odoo/odoo/commit/6b857b6eeb59137a71385f98c82c440ac82cd45d closes odoo/odoo#115159 X-original-commit: 3c6b9249482454a931f384ffc66ceec64dffbcfc Signed-off-by: David Monjoie (dmo) --- addons/web_editor/models/ir_ui_view.py | 17 ++--------------- addons/website/static/tests/tours/rte.js | 2 +- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/addons/web_editor/models/ir_ui_view.py b/addons/web_editor/models/ir_ui_view.py index 8b69d6e02ec..8a3459b9b06 100644 --- a/addons/web_editor/models/ir_ui_view.py +++ b/addons/web_editor/models/ir_ui_view.py @@ -74,7 +74,7 @@ class IrUiView(models.Model): vals = { 'inherit_id': self.id, 'name': '%s (%s)' % (self.name, el.get('id')), - 'arch': self._pretty_arch(arch), + 'arch': etree.tostring(arch, encoding='unicode'), 'key': '%s_%s' % (self.key, el.get('id')), 'type': 'qweb', 'mode': 'extension', @@ -88,19 +88,6 @@ class IrUiView(models.Model): def _save_oe_structure_hook(self): return {} - @api.model - def _pretty_arch(self, arch): - # remove_blank_string does not seem to work on HTMLParser, and - # pretty-printing with lxml more or less requires stripping - # whitespace: http://lxml.de/FAQ.html#why-doesn-t-the-pretty-print-option-reformat-my-xml-output - # so serialize to XML, parse as XML (remove whitespace) then serialize - # as XML (pretty print) - arch_no_whitespace = etree.fromstring( - etree.tostring(arch, encoding='utf-8'), - parser=etree.XMLParser(encoding='utf-8', remove_blank_text=True)) - return etree.tostring( - arch_no_whitespace, encoding='unicode', pretty_print=True) - @api.model def _are_archs_equal(self, arch1, arch2): # Note that comparing the strings would not be ok as attributes order @@ -207,7 +194,7 @@ class IrUiView(models.Model): old_arch = etree.fromstring(self.arch.encode('utf-8')) if not self._are_archs_equal(old_arch, new_arch): self._set_noupdate() - self.write({'arch': self._pretty_arch(new_arch)}) + self.write({'arch': etree.tostring(new_arch, encoding='unicode')}) @api.model def _view_get_inherited_children(self, view): diff --git a/addons/website/static/tests/tours/rte.js b/addons/website/static/tests/tours/rte.js index eda5f989405..41092c0216b 100644 --- a/addons/website/static/tests/tours/rte.js +++ b/addons/website/static/tests/tours/rte.js @@ -228,7 +228,7 @@ wTourUtils.registerWebsitePreviewTour('rte_translator', { content: "Check that the editor is not showing translated content (1)", trigger: '.ace_text-layer .ace_line:contains("an HTML")', run: function (actions) { - var lineEscapedText = $(this.$anchor.text()).text(); + var lineEscapedText = $(this.$anchor.text()).last().text(); if (lineEscapedText !== "<b></b> is an HTML tag & is empty") { console.error('The HTML editor should display the correct untranslated content'); $('iframe:not(.o_ignore_in_tour)').contents().find('body').addClass('rte_translator_error');