The on() method is not properly working on dojox mobile editable widget ( e.g. : List and IconContainer and maybe some others)

[Clement Mathieu]

How to reproduce :

_ use a dojox/mobile/IconContainer with some icons and with the property "editable=true" like in dojox/mobile/tests/test_IconContainer-editable.html
_ Then try to replace the connect.connect(ic, "onDeleteItem", null, function(){}) by ic.on("deleteItem", function(){})
_ delete an item, the callback is not executed.

Cause

The ic.on call does not work properly because it tries to find a function onDeleteItem in the protoype of ic. But since this function is added to ic with a safeMixin, the function is added directly to the object not to its prototype.

[Sebastien Brunot]

update the _onMap of a widget constructor after dynamically adding an editable mixin (IBM CCLA).

[Sebastien Brunot]

I've added a patch to fix the issue.

[Adrian Vasiliu]

sbrunot, could you please add a test? (manual or, better, automatic) Update: as a matter of fact, a manual test case is already included, sorry. (This modifies an existing test to use widget.on() instead of connect.connect(), but since there are many other IconContainer tests that still use connect.connect() both use-cases are covered.)

[Adrian Vasiliu]

See also the discussion in the duplicate #17161.

We now have to make our minds whether we go for the submitted ticket16709.patch, waiting to get rid of it in 2.0, as said in #17161. The concern here is the use of undocumented/private dijit APIs, and the "hacky"/fragile aspect of it. Bill, if you can take a look at it, your advice would help.

[Adrian Vasiliu]

#17161 is a duplicate of this ticket.

[bill]

Replying to Adrian:

We now have to make our minds whether we go for the submitted ticket16709.patch, waiting to get rid of it in 2.0, as said in #17161. The concern here is the use of undocumented/private dijit APIs, and the "hacky"/fragile aspect of it. Bill, if you can take a look at it, your advice would help.

I looked at the patch. It scares me a little because it modifies this.constructor._onMap, affecting all instances of IconContainer or RoundRectList, regardless of whether or not they mix in editableMixinClass. But I guess it will work.

The pre-patch code also scares me, as I worry that declare.safeMixin() is not a valid way to add methods to a class instance, especially methods that call this.inherited(). The safe way is to do what data-dojo-mixins does: create a new subclass.

So, as I said in #17161, my instinct is to close this as wontfix and mark the editable and editableMixinClass properties as deprecated. But if you feel strongly about getting editableMixinClass to work again, you can check in the patch. It seems fragile, but I think it will work.

[Patrick Ruzand]

For the very same reasons as bill ones, I think it's safer not to fix it, and rely on the data-dojo-mixins workaround, which solves the issue also in this case:

<ul id="iconContainer" data-dojo-type="dojox.mobile.IconContainer" data-dojo-mixins="dojox/mobile/_EditableIconMixin" data-dojo-props="editable:true">

(note that 'editable' still need to be set to true, but the dynamic mixin will not be done).

[Adrian Vasiliu]

Ok, thanks bill and pruzand for your input. Since we all think the same, and there is a workaround, and Dojo Mobile 2.0 will anyway solve it in a better way, I'm closing as "wontfix".