From 94018575c6ac21fc94591c3265e017c769d87c0c Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 2 Sep 2021 12:42:53 +0000 Subject: [PATCH] [FIX] web: issues with bounding rects and mouse hit locations On my system in chrome 92 there are issues of chrome's hitboxes (and rounding) being off leading to tests failures, something which given the adjustment in `triggerMouseEvent` had been an issue in the past. The first test failure was / is in the "correctly display year view" calendar test, where the very first event dialog would fail to open when clicking on the `2016-11-16`. After tracing through fullcalendar, it turned out fullcalendar was told to hit `2016-11-*09*`, suspiciously 7 days before the cell we were looking for in a monthly calendar, hinting to an incorrect vertical offset / hitbox. Fix by offsetting the top coordinate in `triggerMouseEvent`, but rather than just increment by one update the entire thing by trigger the event right in the middle of the element being targeted, that seems more reliable long-term, though it could be slightly confusing for events like mouseenter/mouseover/... The second issue was with a list test, where two cells are supposed to be sized equally. Apparently the offset sizes are supposed to be the rounded version of the actual size (which can be fractional), but dumping the information of `foo` and `text` I'd get real widths (via getBoundingClientRect) of respectively 480.46875 and 480.5, and offsetWidths of 481 and 480 (the smaller of the two in "real size" is rounded higher than the other one). In Python we might use `assertAlmostEqual` for this, but we've apparently not introduced this in JS take the real width and round up by hand, this fixes the issue as both end up at 481. Aside 1: Before finding out the actual root cause I landed in `_onYearDateClick` with missing data in the events so looked at the API to figure what I was looking at, and turns out fullcalendar's event object has documented properties to get the date-converted start and end bounds of the event, so use that directly instead of doing the same thing by hand. Aside 2: The attributes passed to `triggerEvent` in `triggerMouseEvent` are pretty much nonsensical, we're getting the element's location using `getBoundingClientRect` which is based off of the viewport, but setting the keys * `screen(X|Y)`, which should be the location of the hit relative to *the physical screen* * `page(X|Y)`, which should be the location of the hit relative to the page / document (so takes in account the content scrolled off-screen) * `layer(X|Y)`, which is the position of the hit relative to the nearest positioned (non-static) element, falling back to page if there isn't one The one attribute which matches the semantics / reference of `getBoundingClientRect` we didn't even set, instead it would be automatically copied from `pageX`/`pageY` by the events mapping. Fix that by just providing `clientX`/`clientY` and updating the mapping functions to not overwrite it. If the others are needed they should be computed properly based on the document's bounding client rect and scroll information, as well as the window's screenX/screenY (and offsetParent for layerX/layerY though that seems unlikely to be necessary). closes odoo/odoo#75953 X-original-commit: d26a4edffd6bdaed46143a30e99f17eb91f44e7e Signed-off-by: Xavier Morel (xmo) --- .../static/src/legacy/js/libs/fullcalendar.js | 8 +++---- .../tests/legacy/helpers/test_utils_dom.js | 21 ++++++++----------- .../static/tests/legacy/views/list_tests.js | 8 +++++-- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/addons/web/static/src/legacy/js/libs/fullcalendar.js b/addons/web/static/src/legacy/js/libs/fullcalendar.js index 8101a4d2ffa..6fa72e016cf 100644 --- a/addons/web/static/src/legacy/js/libs/fullcalendar.js +++ b/addons/web/static/src/legacy/js/libs/fullcalendar.js @@ -219,12 +219,10 @@ odoo.define('/web/static/src/js/libs/fullcalendar.js', function () { */ _onYearDateClick(info) { const calendar = this.context.calendar; - const events = Object.values(this.events) + const events = this.events .filter(event => { - const startUTC = calendar.dateEnv.toDate(event._instance.range.start); - const endUTC = calendar.dateEnv.toDate(event._instance.range.end); - const start = moment(startUTC); - const end = moment(endUTC); + const start = moment(event.start); + const end = moment(event.end); const inclusivity = start.isSame(end, 'day') ? '[]' : '[)'; return moment(info.date).isBetween(start, end, 'day', inclusivity); }) diff --git a/addons/web/static/tests/legacy/helpers/test_utils_dom.js b/addons/web/static/tests/legacy/helpers/test_utils_dom.js index 94b6a814b05..9045377ab0e 100644 --- a/addons/web/static/tests/legacy/helpers/test_utils_dom.js +++ b/addons/web/static/tests/legacy/helpers/test_utils_dom.js @@ -22,15 +22,15 @@ odoo.define('web.test_utils_dom', function (require) { const mouseEventMapping = args => Object.assign({}, args, { bubbles: true, cancelable: true, - clientX: args ? args.pageX : undefined, - clientY: args ? args.pageY : undefined, + clientX: args ? args.clientX || args.pageX : undefined, + clientY: args ? args.clientY || args.pageY : undefined, view: window, }); const mouseEventNoBubble = args => Object.assign({}, args, { bubbles: false, cancelable: false, - clientX: args ? args.pageX : undefined, - clientY: args ? args.pageY : undefined, + clientX: args ? args.clientX || args.pageX : undefined, + clientY: args ? args.clientY || args.pageY : undefined, view: window, }); const touchEventMapping = args => Object.assign({}, args, { @@ -520,14 +520,11 @@ odoo.define('web.test_utils_dom', function (require) { throw new Error(`no target found to trigger MouseEvent`); } const rect = el.getBoundingClientRect(); - // little fix since it seems on chrome, it triggers 1px too on the left - const left = rect.x + 1; - const top = rect.y; - return triggerEvent($el, type, { - which: 1, - pageX: left, layerX: left, screenX: left, - pageY: top, layerY: top, screenY: top, - }); + // try to click around the center of the element, biased to the bottom + // right as chrome messes up when clicking on the top-left corner... + const left = rect.x + Math.ceil(rect.width / 2); + const top = rect.y + Math.ceil(rect.height / 2); + return triggerEvent(el, type, {which: 1, clientX: left, clientY: top}); } /** diff --git a/addons/web/static/tests/legacy/views/list_tests.js b/addons/web/static/tests/legacy/views/list_tests.js index a6e8d978235..54ea0ac83e7 100644 --- a/addons/web/static/tests/legacy/views/list_tests.js +++ b/addons/web/static/tests/legacy/views/list_tests.js @@ -3152,8 +3152,12 @@ QUnit.module('Views', { arch: '', }); - const fooWidth = list.$('th[data-name="foo"]')[0].offsetWidth; - const textWidth = list.$('th[data-name="text"]')[0].offsetWidth; + const foo = list.el.querySelector('th[data-name="foo"]'); + const fooWidth = Math.ceil(foo.getBoundingClientRect().width); + + const text = list.el.querySelector('th[data-name="text"]'); + const textWidth = Math.ceil(text.getBoundingClientRect().width); + assert.strictEqual(fooWidth, textWidth, "both columns should have been given the same width"); const firstRowHeight = list.$('.o_data_row:nth(0)')[0].offsetHeight;