Button: Cannot stop click event from bubbling

[Paul Christopher]

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.

[bill]

#15099 is a duplicate of this ticket.

[bill]

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];
}

[Paul Christopher]

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).

[bill]

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().

[Douglas Hays]

possible fix

[Douglas Hays]

Paul Christopher, can you please test with the attached patch to see if it behaves as you expect?

[Douglas Hays]

refreshed fix

[Douglas Hays]

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.

[Paul Christopher]

Just tested the new, revised patch with the above test case, and - as far as I can judge - it works perfectly now!

[Douglas Hays]

In [29962]:

Fixes #15097. Stop the initial click on the visible button-like elements and fire a synthetic click on the hidden typed INPUT so that native event behavior works as expected. Utilize the new event.defaultPrevented to stop calls to _onSubmit for non-form widgets. Added automated tests compare the behavior to native buttons.

[bill]

Just noting the dependency for the record.

[bill]

In [29964]:

add comment for 2.0, refs #15097 tangentially, !strict

[bill]

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)

[Douglas Hays]

In [29994]:

Refs #15097. Wrap valueNode.click() inside defer() so the current event that is being stopped doesn't get mixed up in IE. Fixed several tests to not use fake click events that don't understand defaultPrevented. Fixed several tests to not assume click() processing is synchronous. Added additional mobile tests to the mixin tests.

[bill]

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.

[Douglas Hays]

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.

[bill]

In [30051]:

When dojo/on compares the current event to a previously saved event, check if evt.target matches too. This avoids false matches in the case of dijit/form/Button, where a click event on a <span> programatically triggers a click event on another node (an <input type=button>). Fixes #15097 !strict.

Also rolls back [29994] which is no longer needed.

[bill]

In [30052]:

Fix StackContainer test on IE, refs #15097

[Douglas Hays]

In [30071]:

Refs #15097. IE < 9 was not correctly processing checkbox onclick event handlers when on() was used with the click() method.

[bill]

There's still an issue with dojo/on. I split it off to #16543.

[bill]

In [30281]:

Evt.target is not set for native events on IE6-8, so need to use evt.srcElement instead. Fixes regression in dojo/tests/on/on from [30051], refs #15097 and fixes #16543 !strict.

[Bill Keese <[email protected]…>]

In 45637a1860c3cbc5e462511aa8fd94e58fc3ffbf/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Bill Keese <[email protected]…>]

In f6a11a5a3dd182eb5812fd036032c01eb38ce8d6/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Bill Keese <[email protected]…>]

In 8a7b57d877c1a5335359bcaacf1cc47bde79e76b/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Bill Keese <[email protected]…>]

In 4ef225abfcf5fad732501e5c154edd07bb6068a2/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Bill Keese <[email protected]…>]

In ab2d0ca45bfe35b2433b44ee3c914af141083bf1/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Bill Keese <[email protected]…>]

In e12545073e958fd81eae0199a10192f0bd8ca287/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Bill Keese <[email protected]…>]

In 531fe6db1799718ae4f79186795df9d4a6db10db/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Bill Keese <[email protected]…>]

In a4e7826a8561eed9f5084e1e17796b1f94ff0345/dijit:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled?