Mithril.js: A blocked redraw from an event handler lower in the tree cascades upwards.

[dwaltrip]

If two nested divs both have onclick handlers, and the inner div blocks the redraw with e.redraw = false, then neither of the onclick handlers will result in a redraw.

It seems that the browser passes the same event object to all handlers up the tree, as part of the bubbling process. Thus, both handlers will have an event object with redraw set to false, although it was only set to false in the inner handler.

This cascading of blocked redraws only occurs in one direction, up the tree. If, instead, we set e.redraw = false in the outer handler, then a click event on the inner div will trigger a redraw after the inner handler, but not again after the outer handler.

This surprised me at first. In some situations, it creates the unfortunate requirement of needing to know whether event handlers lower down in the tree are blocking the redraw.

I'm not sure what the ideal behavior would be, but wanted to at least document the issue. Adding a note to the official documentation might be all that is needed.

Perhaps the more important lesson is to generally avoid designs or logic that depends on redraws being blocked, and use event.redraw = false only if absolutely needed.

Here's a codepen to illustrate: https://codepen.io/anon/pen/xdYdXM?editors=0011

[pygy]

Thanks for the report, this is at the very least a gotcha, and IMO a bug that should be fixed.

We could set event.redraw to true before passing it to the handler. That way, setting it to false in a handler will not contaminate other ones "uptree".

[gyandeeps]

I can do the work if nobody is working on this.

[dwaltrip]

I've linked the lines below I identified as relevant when I was looking into it =) I think the fix recommended by @pygy might be a one-liner? Of course then we have to make sure everything else works as expected. I'm guessing this doesn't affect the rest of the render/render.js code, but then again I don't understand the nitty gritty details of the core renderer very well.

https://github.com/MithrilJS/mithril.js/blob/next/api/redraw.js#L28-L30
https://github.com/MithrilJS/mithril.js/blob/next/render/render.js#L15
https://github.com/MithrilJS/mithril.js/blob/next/render/render.js#L566

[pygy]

The fix is a one liner, yes. I'd reset e.redraw to undefined in api/redraw.js, in order to keep render/render.js redraw-unaware.

We'll also need a test. I don't know if the DOM mock code implements event bubbling. If it doesn't checking that e.redraw has been reset after a timeout is better than nothing.

Edit: @gyandeeps thanks for the offer, PR most welcome!

[isiahmeadows]

@pygy

I don't know if the DOM mock code implements event bubbling.

It currently does not. But once we get sync redraws, this'll be much easier to test.