From 55b9e4c588fbb7f1fd731fc60fc1ce8965ab0143 Mon Sep 17 00:00:00 2001 From: Benoit Socias Date: Thu, 19 May 2022 12:29:03 +0000 Subject: [PATCH] [FIX] web_editor: compare size CSS properties When CSS properties are updated, a check is made to verify whether the new value is different from the old one. This process sometimes involves unit conversions. Since [1] when a CSS property is updated, it is also set before the check, in order to verify whether it should be set as `!important`. When the CSS property is a size, such as the `background-size` CSS property, it is made of two dimensions. However the verification does not handle that situation: it handles `15px auto` as being a single number with the unit `px auto`, which is wrong. The full comparison is made because the strings do not fully match: when a size property is set to a measure followed by `auto`, it is returned without `auto` when reading the computed style. E.g. after setting `15px auto`, it is read as `15px`. This commit adapts the CSS properties comparison mechanism by splitting the size properties into their parts and comparing each part separately. This commit also introduces a few tests about the already existing CSS properties comparisons. Steps to reproduce: - Drop a "Banner" block. - Set "Position" as "Repeat Pattern". - Type "5" in the "Width". => It raised an error. [1]: https://github.com/odoo/odoo/commit/d3c3dab8950abc25b29937605091d8ce32305fa4 task-2853161 X-original-commit: 07e471b9398e042c45182f1e8ca33e74dd8087a3 Part-of: odoo/odoo#109078 --- .../web_editor/static/src/js/common/utils.js | 17 ++++ addons/web_editor/static/tests/utils_tests.js | 77 +++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 addons/web_editor/static/tests/utils_tests.js diff --git a/addons/web_editor/static/src/js/common/utils.js b/addons/web_editor/static/src/js/common/utils.js index 16de3f2dc23..3049eb706c0 100644 --- a/addons/web_editor/static/src/js/common/utils.js +++ b/addons/web_editor/static/src/js/common/utils.js @@ -172,6 +172,23 @@ function _areCssValuesEqual(value1, value2, cssProp, $target) { return true; } + // In case the values are a size, they might be made of two parts. + if (cssProp && cssProp.endsWith('-size')) { + // Avoid re-splitting each part during their individual comparison. + const pseudoPartProp = cssProp + '-part'; + const re = /-?[0-9.]+\s*[A-Za-z%-]+|auto/g; + const parts1 = value1.match(re); + const parts2 = value2.match(re); + for (const index of [0, 1]) { + const part1 = parts1 && parts1.length > index ? parts1[index] : 'auto'; + const part2 = parts2 && parts2.length > index ? parts2[index] : 'auto'; + if (!_areCssValuesEqual(part1, part2, pseudoPartProp, $target)) { + return false; + } + } + return true; + } + // It could be a CSS variable, in that case the actual value has to be // retrieved before comparing. if (value1.startsWith('var(--')) { diff --git a/addons/web_editor/static/tests/utils_tests.js b/addons/web_editor/static/tests/utils_tests.js new file mode 100644 index 00000000000..2d53e6e48d1 --- /dev/null +++ b/addons/web_editor/static/tests/utils_tests.js @@ -0,0 +1,77 @@ +/** @odoo-module **/ +import weUtils from 'web_editor.utils'; + +QUnit.module('web_editor', {}, function () { +QUnit.module('utils', {}, function () { + QUnit.module('Utils functions'); + // Test weUtils + + QUnit.test('compare CSS property values', async function (assert) { + assert.expect(74); + + let $div = $(`
abc
`); + let currentProperty = 'font-size'; + function compare(expected, value1, value2, property = currentProperty, $target = $div) { + // Comparisons are done in both directions if values are different. + assert.strictEqual(weUtils.areCssValuesEqual(value1, value2, property, $target), expected, + `'${value1}' should be ${expected ? 'equal to' : 'different from'} '${value2}'` + ); + if (value1 !== value2) { + assert.strictEqual(weUtils.areCssValuesEqual(value2, value1, property, $target), expected, + `'${value2}' should be ${expected ? 'equal to' : 'different from'} '${value1}'` + ); + } + } + compare(true, '', ''); + compare(true, 'auto', 'auto'); + compare(true, '', 'auto'); + compare(true, '15px', '15px'); + compare(true, '15.0px', '15px'); + compare(true, '15 px', '15px'); + compare(false, '15px', '25px'); + compare(false, '15px', ''); + compare(false, '15px', 'auto'); + + currentProperty = 'background-size'; + compare(true, '', ''); + compare(true, 'auto', 'auto'); + compare(true, '', 'auto'); + compare(true, '15px', '15px'); + compare(true, '15.0px', '15px'); + compare(false, '15px', '25px'); + compare(false, '15px', ''); + compare(false, '15px', 'auto'); + compare(true, '', 'auto auto'); + compare(true, 'auto', 'auto auto'); + compare(true, 'auto auto', 'auto auto'); + compare(true, '15px 15px', '15px 15px'); + compare(false, '15px 25px', '15px 15px'); + compare(false, '25px 15px', '15px 15px'); + compare(true, 'auto 15px', 'auto 15px'); + compare(true, '15px auto', '15px auto'); + compare(true, '15px', '15px auto'); + compare(false, 'auto 25px', 'auto 15px'); + compare(false, '25px auto', '15px auto'); + compare(false, '25px', '15px auto'); + + currentProperty = 'color'; + compare(true, '', ''); + compare(false, '', '#123456'); + compare(true, '#123456', '#123456'); + compare(false, '#123456', '#654321'); + compare(true, 'rgb(255, 0, 0)', '#FF0000'); + compare(false, 'rgb(255, 0, 0)', '#EE0000'); + + currentProperty = 'background-image'; + compare(true, '', ''); + compare(false, '', 'linear-gradient(0deg, rgb(0, 0, 0) 0%, rgb(1, 1, 1) 100%)'); + compare(true, 'linear-gradient(0deg, rgb(0, 0, 0) 0%, rgb(1, 1, 1) 100%)', 'linear-gradient(0deg, rgb(0, 0, 0) 0%, rgb(1, 1, 1) 100%)'); + compare(false, 'linear-gradient(0deg, rgb(10, 0, 0) 0%, rgb(1, 1, 1) 100%)', 'linear-gradient(0deg, rgb(0, 0, 0) 0%, rgb(1, 1, 1) 100%)'); + compare(false, 'linear-gradient(10deg, rgb(0, 0, 0) 0%, rgb(1, 1, 1) 100%)', 'linear-gradient(0deg, rgb(0, 0, 0) 0%, rgb(1, 1, 1) 100%)'); + compare(false, 'linear-gradient(0deg, rgb(0, 0, 0) 0%, rgb(10, 1, 1) 100%)', 'linear-gradient(0deg, rgb(0, 0, 0) 0%, rgb(1, 1, 1) 100%)'); + compare(false, 'linear-gradient(0deg, rgb(0, 0, 0) 0%, rgb(9, 9, 9) 50%, rgb(1, 1, 1) 100%)', 'linear-gradient(0deg, rgb(0, 0, 0) 0%, rgb(1, 1, 1) 100%)'); + compare(true, 'linear-gradient(0deg, #000000 0%, #010101 100%)', 'linear-gradient(0deg, rgb(0, 0, 0) 0%, rgb(1, 1, 1) 100%)'); + compare(false, 'linear-gradient(0deg, #FF0000 0%, #010101 100%)', 'linear-gradient(0deg, rgb(0, 0, 0) 0%, rgb(1, 1, 1) 100%)'); + }); +}); +});