Regression with widgets in template having attach events

[Brian Arnold]

I'm providing both a demonstration and a one-line patch on this ticket, which shows the issue we've recently encounter with templated widgets that also have widgets in their template.

If a templated widget contains a widget, and tries to use an attach event to bind to that event, if that event is one that comes in from dijit/_Widget's DOM-like events, the event does not bind correctly.

After doing some digging, it looks like it's how back-compat was implemented in dijit/_WidgetsInTemplateMixin. In _beforeFillContent, it follows this logic when dealing with attach events:

        // callback to do data-dojo-attach-event to a widget
        if(type in widget){
                // back-compat, remove for 2.0
                return aspect.after(widget, type, callback, true);
        }else{
                // 1.x may never hit this branch, but it's the default for 2.0
                return widget.on(type, callback, true);

In the event of using something like this:

<div data-dojo-type="dijit/form/TextBox" data-dojo-attach-event="onClick: doSomething"></div>

It'll fall into that first branch and use aspect.after. However, aspect.after isn't a valid replacement for the older connect method from dojo/_base/connect. Semantically they're similar, but dijit/_Widget is adding an aspect around connect.connect and kernel.connect that vivifies the event upon binding, and aspect.after is left clean, so the event is bound but it's never actually fired.

In the demo I'm uploading, you'll notice that I'm using both a Button and a TextBox?. The Button fires its onClick, but that's because dijit/form/_ButtonMixin explicitly defines an onClick method, rather than leaning on the automagic one from dijit/_Widget. The TextBox?, on the other hand, doesn't ever actually fire its onClick since it wasn't brought to life.

The fix is a simple one. It's a one-line change that seems to be working well, and is valid since it's a back-compat adjustment anyways:

        // callback to do data-dojo-attach-event to a widget
        if(type in widget){
                // back-compat, remove for 2.0
                return widget.connect(widget, type, callback);
        }else{
                // 1.x may never hit this branch, but it's the default for 2.0
                return widget.on(type, callback, true);

By replacing the aspect with widget.connect, the event comes to life as it used to in 1.8 and prior.

The attached demonstration of the bug is a simple bugdemo package, to be put as a sibling of dojo etc. Works well out of the box with 1.8, completely broken against current trunk. With this one-line adjustment applied locally, everything works well again.

I'm also attaching a patch file that cleanly applies against trunk right now to make the change.

Normally I wouldn't feel bad about making a one-line change, but given that it's in the heart of a fairly important Dijit, I figured I'd pass it through a ticket for your review, Bill. Thoughts?

[Brian Arnold]

Demonstration of widgets in template with attach events not working well against Dojo trunk

[Brian Arnold]

A one-line patch for _WidgetsInTemplateMixin to restore behavior of widgets with attach events

[bill]

Thanks, that all makes sense. Feel free to check in if you can add a samll test into _WidgetsInTemplateMixin.html. I guess you would need to on.emit() an event to tst that the hookup really worked.

[Brian Arnold]

In [31268]:

Changing how back-compat is handle for attach events, fixes #17002