#15097 closed defect (fixed)
Button: Cannot stop click event from bubbling
Reported by: | Paul Christopher | Owned by: | Douglas Hays |
---|---|---|---|
Priority: | undecided | Milestone: | 1.9 |
Component: | Dijit - Form | Version: | 1.7.2 |
Keywords: | Cc: | Kitson Kelly, ben hockey | |
Blocked By: | #16344 | Blocking: |
Description
Description
Calling stopPropagation e.g. in a (dijit) click handler should stop the propagation of a click event for regular (dom) click handlers, too.
Test Case
Have a look at the attached test case. It is a simple div containing a dijit button. You can select the div by clicking on it, but clicking the button should perform some action and not select the div. Therefore I tried to call stopPropagation in the button's click handler - without any effect.
Workaround
The workaround is to set up a separate dom-click-handler on the button's domNode and call stopPropagation there.
Discussion
This is not a real bug report nor a real feature request, but more a real life example(?? or a newbie's mistake???). I just place it here so as to make sure that this cross-dom-dijit-event case is considered in system design. Myself, I'm not absolutely sure, whether it is a good idea to cancel all dom-clicks when a dijit-click with stopPropagation is called. But calling both events the same ('click') could be sometimes confusing, too.
Attachments (3)
Change History (32)
Changed 9 years ago by
Attachment: | testStopPropagation.html added |
---|
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
Cc: | Kitson Kelly added |
---|---|
Component: | General → Dijit |
Milestone: | tbd → 2.0 |
Owner: | set to bill |
Status: | new → assigned |
I guess agree that your suggested behavior would be nicer than the current behavior, and I think it will be fixed in 2.0 when all widget event connections just get delegated to be connections events on the DOM. I don't see a practical way to fix it before then.
I'm referring to removing the "backwards-compatibility" section in the current definition of _WidgetBase.on():
on: function(/*String*/ type, /*Function*/ func){ // summary: // Call specified function when event occurs, ex: myWidget.on("click", function(){ ... }). // description: // Call specified function when event `type` occurs, ex: `myWidget.on("click", function(){ ... })`. // Note that the function is not run in any particular scope, so if (for example) you want it to run in the // widget's scope you must do `myWidget.on("click", lang.hitch(myWidget, func))`. // For backwards compatibility, if there's an onType() method in the widget then connect to that. // Remove in 2.0. var widgetMethod = this._onMap(type); if(widgetMethod){ return aspect.after(this, widgetMethod, func, true); } // Otherwise, just listen for the event on this.domNode. return this._adoptHandles(on(this.domNode, type, func))[0]; }
comment:3 Changed 9 years ago by
This ticket might be related to http://bugs.dojotoolkit.org/ticket/14734, which aims at providing a a11y click for DOM elements. Strictly speaking: A simple DOM click is not really what I want for the div. The div should be selectable by keyboard, too (provided that it has the appropriate tabindex attribute).
comment:4 Changed 8 years ago by
Component: | Dijit → Dijit - Form |
---|---|
Milestone: | 2.0 → tbd |
Owner: | changed from bill to Douglas Hays |
Summary: | Cannot stop click event from bubbling → Button: Cannot stop click event from bubbling |
Actually in this case I think the problem is in Button.js. The original click event is getting canceled, but then Button.js calls _onClick which creates another event which bubbles:
_onClick: function(/*Event*/ e){ // summary: // Internal function to handle click actions var ok = this.inherited(arguments); if(ok){ if(this.valueNode){ this.valueNode.click(); e.preventDefault(); // cancel BUTTON click and continue with hidden INPUT click e.stopPropagation(); // avoid two events bubbling from Button widget // leave ok = true so that subclasses can do what they need to do } } return ok; },
If stopPropagation() was called on the original event, then it should be called on the new event. Likewise for preventDefault().
comment:5 Changed 8 years ago by
Milestone: | tbd → 1.9 |
---|---|
Type: | enhancement → defect |
comment:6 Changed 8 years ago by
Paul Christopher, can you please test with the attached patch to see if it behaves as you expect?
comment:7 Changed 8 years ago by
I added a refreshed patch that uses bill's suggestion to stop the initial button click and then only deal with the synthetic event on the hidden valueNode. This needs review and testing. I also added automated tests that compare native button click event handling to both Button and Dialog/submit handling.
comment:8 Changed 8 years ago by
Just tested the new, revised patch with the above test case, and - as far as I can judge - it works perfectly now!
comment:10 Changed 8 years ago by
Blocked By: | 16344 added |
---|
Just noting the dependency for the record.
comment:12 Changed 8 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Hmm, I'm getting lots of failures after [29962], for example ButtonMixin.html. Failing on IE and FF.
A partial list of the IE8 failures is:
- robot/Dialog_a11y
- robot/Toolbar
- Button (form/test_Button.html?mode=test)
- Button_a11y
- CheckBoxMixin (onclick w3)
- ButtonMixin (events)
comment:14 Changed 8 years ago by
Thanks for working on that. I'm seeing failures now though in for example in Dialog_mouse.html, because the submit button doesn't close the dialog anymore.
comment:15 Changed 8 years ago by
Cc: | ben hockey added |
---|
comment:16 Changed 8 years ago by
This seems to be a problem with IE specific code within on.js. A global lastEvent is being used and 2 different type==click events are arriving nearly simultaneously on 2 distinct nodes but the on._fixEvent processing is assuming they are the same event. Artificially separating the click events by 1 or more milliseconds seems to work around the issue but I'd rather not do that. The defer() in _ButtonMixin.js should be removed after this is fixed in on.js.
#15099 is a duplicate of this ticket.