[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
This commit is contained in:
Benoit Socias
2023-01-05 11:35:52 +01:00
parent 068e47363a
commit 55b9e4c588
2 changed files with 94 additions and 0 deletions
@@ -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(--')) {
@@ -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 = $(`<div>abc</div>`);
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%)');
});
});
});