Skip to content
Snippets Groups Projects
Commit 68794147 authored by qsm-odoo's avatar qsm-odoo
Browse files

[FIX] web: properly listen to colorpicker drag events

With [1] and [2], the colorpicker / colorpalette were implemented. In
both cases, for the custom colors hue / saturation / lightness selection
we listened to mousemove/mouseup events on the whole body / document.
That is to be able to start dragging the color cursors from inside the
colorpicker to anywhere on the screen (the cursor being confined into
their own box of course).

Problems occur when iframes come into the equation. Indeed, from that
moment, listening to mousemove events on an unique document is not the
way to go as mousemove over inner iframes will be ignored. This should
not be the case anywhere in 15.0 (that this original commit targets).
The only iframe there should be the mass_mailing one and in that case,
the colorpicker is inside the iframe so it should not be a problem.
However, [3] (then fixed by [4]) were meant to tackle a similar problem
(although [3] seems to say the goal was to ignore mousemove outside the
iframe which is the opposite of what this current commit tries to do:
listening on the whole screen). However, by doing so, a memory leak was
created as the document on which the events were bound was not properly
cleaned as the `destroy` which unbinds the events was not adapted. That
is the main reason why this commit targets 15.0 (although, the solution
here still makes sense generically in 15.0 and should make the code
simpler and easier to understand).

In 16.0, however, the iframe situation is a problem: the website builder
colorpickers are now in the main document but the edited content is
inside an inner iframe. In that case, listening to the main document's
mousemove/mouseup events makes the colorpicker only work when not
hovering the edited content (which could be acceptable but not perfect).
Note that during that website builder refactoring, [5] worsened the code
of [3] and [4] to do exactly that. Later, [6] was made to solve an
unrelated problem and actually woke up that worsened code, not
understanding what the new undocumented `ownerDocument` option was for
(it will stay like this in stable from now on, as it will be not used
anywhere anymore... hopefully). Because of that, the situation became
the opposite: dragging inside the colorpicker did not work until the
edited content was hovered. And because of that functional fact, an even
more visible problem was created:

- Click on a snippet
- Select a custom background color using the hsl selection
- Hover a non custom color (like nearly always the case "by mistake"
  after configuring a custom color, your mouse just moves over the non
  custom colors) => your custom color is lost

This was because the chain of events that were listened inside the
colorpicker were gone (because of the combination of [5] and [6]). In
the end, this only revealed the shortcomings of the implementation in
[1], [2], [3] and [4]: we should not try to search which frame we have
to listen to: we should listen to the whole screen. This commit solves
the issue by listening to the top window document and all its inner
iframes, also fixing the memory leak mentioned earlier.

task-3332711

[1]: https://github.com/odoo/odoo/commit/29f0c0a186a9a60d6e5b7f026c305d689b6fc807
[2]: https://github.com/odoo/odoo/commit/99910b526ed792235a778863b993d10cf4e77cd7
[3]: https://github.com/odoo/odoo/commit/4fb33f7f8d6955a44fbb7c94c7d204e9baa09707
[4]: https://github.com/odoo/odoo/commit/f1b26335a286a47f685b7830db97c21e6fefd68b
[5]: https://github.com/odoo/odoo/commit/03c552690b15cbf2e7d6b7812386ac64042219af
[6]: https://github.com/odoo/odoo/commit/418c1178301e28b4bd1412da6d0558e219060830



closes odoo/odoo#125993

X-original-commit: 63bf363d
Signed-off-by: default avatarQuentin Smetz (qsm) <qsm@odoo.com>
parent a0a7119f
No related branches found
No related tags found
No related merge requests found
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment