Opened 8 years ago
Closed 8 years ago
#17002 closed defect (fixed)
Regression with widgets in template having attach events
Reported by: | Brian Arnold | Owned by: | Brian Arnold |
---|---|---|---|
Priority: | blocker | Milestone: | 1.9 |
Component: | Dijit | Version: | 1.9.0b2 |
Keywords: | Cc: | cjolif, Ed Chatelain | |
Blocked By: | Blocking: |
Description
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?
Attachments (2)
Change History (6)
Changed 8 years ago by
Attachment: | bugdemo.zip added |
---|
Changed 8 years ago by
Attachment: | _WITM-backcompat.patch added |
---|
A one-line patch for _WidgetsInTemplateMixin to restore behavior of widgets with attach events
comment:1 Changed 8 years ago by
Owner: | changed from bill to Brian Arnold |
---|---|
Status: | new → assigned |
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.
comment:2 Changed 8 years ago by
Cc: | cjolif added |
---|
comment:3 Changed 8 years ago by
Cc: | Ed Chatelain added |
---|
Demonstration of widgets in template with attach events not working well against Dojo trunk