Opened 8 years ago

Closed 8 years ago

#13364 closed defect (fixed)

[regression] BusyButton declarative "dojo/connect" broken.

Reported by: Kitson Kelly Owned by: bill
Priority: high Milestone: 1.7
Component: DojoX Form Version: 1.7.0b1
Keywords: Cc:
Blocked By: Blocking:

Description

I have attached an example where the following code does not fire, though the equivalent works perfectly with dijit.form.Button.

<button data-dojo-type="dojox.form.BusyButton" id="testBusyButton" 
    data-dojo-props="busyLabel:&#39;Busy...&#39;,timeout:5000">Test Busy Button
  <script type="dojo/connect" data-dojo-event="onClick" data-dojo-args="evt">
    console.log("dojox.form.BusyButton");
  </script>
</button>

I am not sure when the regression was introduced, but it used to work previously.

Attachments (2)

test_BusyButtonConnect.html (1.4 KB) - added by Kitson Kelly 8 years ago.
Example of Broken Behaviour
13364.diff (309 bytes) - added by Kenneth G. Franqueiro 8 years ago.

Download all attachments as: .zip

Change History (9)

Changed 8 years ago by Kitson Kelly

Attachment: test_BusyButtonConnect.html added

Example of Broken Behaviour

comment:1 Changed 8 years ago by Kitson Kelly

A bit further experimentation shows that no matter what, BusyButton? is not firing onClick, but the private _onClick is firing and if I dojo.connect or dojo.hitch to that, then the code fires. I can't see anything obviously wrong with BusyButton?, but there maybe issues further up the inheritance stack.

comment:2 Changed 8 years ago by Kenneth G. Franqueiro

Milestone: tbd1.7
Owner: changed from dante to bill
Summary: BusyButton declarative "dojo/connect" broken.[regression] BusyButton declarative "dojo/connect" broken.

This is only a guess, but I'm thinking it may have something to do with [23996] - which changed the name of a method and causes it to clobber _onClick, which means that the _onClick from dijit/form/_ButtonMixin, which is responsible for calling onClick, no longer gets called.

Reassigning to Bill since he made that change. I'm not familiar with what broke that he was trying to fix at the time. Not sure whether fixing this is as simple as adding this.inherited(arguments)?

comment:3 Changed 8 years ago by Kitson Kelly

I tried this.inherited(arguments) and return this.inherited(arguments) and neither one caused onClick to fire.

comment:4 Changed 8 years ago by Kitson Kelly

As kgf pointed out on IRC, doing this.inherited(arguments) before this.makeBusy() works, so it appears that this.makeBusy() improperly clobbers _onClick().

comment:5 Changed 8 years ago by Kenneth G. Franqueiro

Well no, _onClick clobbers _onClick. That's how declare (w/o this.inherited) works. :P

But calling this.inherited(arguments) only immediately before calling makeBusy (within the same conditional block) would seem to make sense, since this is ordinarily the only time where the button should perform an action when clicked.

Calling it after makeBusy is called would mean the button is already disabled, which would throw a wrench into the works, as dijit/form/_ButtonMixin's _onClick stops the event dead if it finds the button is disabled.

I'll attach a one-line diff to indicate where I'm talking about.

Changed 8 years ago by Kenneth G. Franqueiro

Attachment: 13364.diff added

comment:6 Changed 8 years ago by bill

Yup, looks like that's all that's needed. FYI, I think what I was fixing with [23996] was that clicking the button had no effect at all.

comment:7 Changed 8 years ago by bill

Resolution: fixed
Status: newclosed

(In [25749]) fix BusyButton to call onClick() when clicked, fixes #13364 !strict

Note: See TracTickets for help on using tickets.