dojo/touch click event normalization and fixes

[Damien Mandrioli]

When you click on a ListItem contained in a ScrollableView, the click event is duplicated.

This affects only Android 4.1 and is related to ​http://code.google.com/p/android/issues/detail?id=36116

Workaround: listen for touchEnd instead of click event or use a View instead of a ScrollableView.

[Eric Durocher]

Do not fire synthetic click events in ScrollableView for Android >= 4.1 - Eric Durocher (IBM, CCLA)

[Eric Durocher]

The ScrollableView fires synthetic click events because it prevents/stops touchstart/move events (to disable native browser scrolling), and in that case the OS normally does not fire click events on touchend. However, this is not true in Android 4.1: the click event is still fired even though touchstart/move events are stopped. This explains why we see duplicate click events. The solution is to not fire click events again on Android 4.1.

This will have to be tested again on future Android versions to see if the behavior changes again...

[cjolif]

In [29617]:

refs #15878. Fixes double click events occuring on Android >= 4.1. Thanks Eric Durocher (IBM, CCLA). !strict.

[cjolif]

In [29619]:

fixes #15922, #15878. Backport fixes from Eric Durocher (IBM, CCLA) into 1.8 branch. !strict.

[Eric Durocher]

Testing the Android version (>= 4.1) does not seem reliable enough, as the click event is apparently not sent on Android 4.1 devices using the Chrome browser (Galaxy Note II). So, I am reopening this and looking for a more general solution not based on version sniffing.

[Eric Durocher]

This other problem seems similar: #16676. Not really a duplicate since it is not in the same module, but the resolution should be similar.

[Eric Durocher]

OK, I'd like to propose this new patch as a browser independent solution.

The idea is the following: instead of trying to guess which browser will / will not send a click after a preventDefault() on touch events, we always generate click events ourselves in dojo/touch, and we always stop native click events sent by the browser.

This should solve several different problems:

• this ticket about dojox/mobile/scrollable, and the similar ticket on mobile Switch: #15922 (with the patch, we can just remove the ugly code that generated the clicks in these widgets)
• a similar ticket on dojox/gesture: #16676
• and also the "click delay" problem: #16720

The patch can probably be enhanced, for example we could probably avoid adding new listeners on the document for touchmove and touchend. I added different listeners because we need to always be notified of touchmove/end events, even if the event was stopped in a listener, and the existing listeners in dojo/touch do not use addEventListener(..., true). The existing listeners could probably be changed to use addEventListener(..., true), thus removing the need for the new listeners. Bill?

Also, not sure about the move threshold (40 currently, this seems approximately what iOS does...)

[bill]

I was also thinking about the click delay, which some people consider a problem. You listed it as #16720 but there's also a lot of prior discussion at ​http://thread.gmane.org/gmane.comp.web.dojo.devel/17903 and #15110. My key point from #15110 is that the delay is by design from Apple to enable page zoom via double click, so we want to be cautious if we are intentionally breaking that behavior.

Your proposed patch is similar to code that's already in dijit/a11yclick.js. The main difference of your code, compared to the code in a11yclick, is that your code is applying this behavior to every node on the page, not just nodes with a11yclick listeners on them.

Your code is better for performance since it just sets up a few listeners on the page rather than listeners on every node, and it possibly fixes problems trying to use event delegation with dijit/a11yclick (ex: putting a dijit/a11yclick handler on a Toolbar instead of the individual buttons). Your code is more dangerous though; developers might not appreciate how merely loading dojo/touch causes side effects on their whole page. A compromise would be to use your design, but only enable that behavior for nodes that have a property (ex: dojoClick) and/or attribute (ex: data-dojo-click-node) or class name set. Perhaps also check if it's set on an ancestor of the node. That's similar to _CssStateMixin's technique. Not sure what's best.

Like the a11yclick code, your code has a bug where zooming the page via doubleclick causes an unexpected click event. Except your bug is presumably worse because it affects doubleclicking anywhere on the page, not just doubleclicking nodes with dijit/a11yclick handlers. Not an issue for pages where zoom is disabled, but it could be an issue for other pages.

Also, you shouldn't be using dojo/_base/event, it's effectively deprecated. Pluswhich, dojo/_base/event::stop() calls evt.stopPropagation() but presumably you want to call evtlstopImmediatePropagation(), in case the clicked node itself has a "click" listener registered.

[bill]

PS: The idea behind a synthetic event like dijit/a11yclick (which perhaps should just be called dijit/click) is that app code call on(node, a11yclick, ...) instead of on(node, "click", ...). If code follows that rule, the on.emit() call doesn't have to emit "click", it could emit "dojoclick" etc., similar to how the other code in dojo/touch works. That would be more straightforward, albeit users would need to be trained to do on(node, a11yclick, ...).

The reasons a11yclick is currently emitting a "click" event are:

1. for native browser behavior. For example, dijit/Checkbox is an actual <input type=checkbox> so we depend on the browser rather than javascript to toggle it.
2. because the app may be listening for "click" itself, rather than dijit/a11yclick

[Eric Durocher]

Yes I thought of a separate dojoclick event too (this could be exposed as touch.click), but did not do it for the same 2 reasons you mention...

About your previous comments, I agree this may be too intrusive and it would probably be safer to explicitly enable this on a specific node/hierarchy only.

[Eric Durocher]

New version of the patch which

• also generates clicks on IE10 (where the delay is noticeable too)
• generates clicks only for nodes that have the dojoClick property set to true (set by dojox/mobile to the whole document by default).
• replaces similar code in dijit/a11yclick by a dojo/touch require

Note, the old dijit/a11yclick code was not strictly equivalent, since it used a timer to wait for a potential native click fired by the browser. To decide if we really want to do this.

[Eric Durocher]

New version of patch where dojoClick is set only on some dojox/mobile widgets where the native behavior is problematic (Buttons, scrollable, Switch). Also added a way to customize the move threshold per node.

[Eric Durocher]

On touch-enabled devices, generate clicks in dojo/touch instead on relying on the browser, to avoid inconsistent behaviors - Eric Durocher (IBM, CCLA)

[Eric Durocher]

Yet another version: reverted to setting dojoClick on the whole body, to get consistent click behavior on all mobile widgets.

[bill]

In [30784]:

Put touch click normalization in dojo/touch, rather than in dijit/a11yclick and various mobile modules. Unlike the old dijit/a11yclick code, the new code works by squelching the real click event, and instead always generating a synthetic click event. Limited to nodes that have a dojoClick property set, or an ancestor with the dojoClick property set. Patch from Eric Durocher (IBM, CCLA), fixes #15878 !strict.

Some dijit widgets like dijit/Tree and dijit/TitlePane are currently manually listening for click events and keyboard events; they need to be updated to set the dojoClick property on their nodes instead.

[bill]

In [30795]:

remove unused dependencies and streamline a11yclick code, refs #15878 !strict

[Eric Durocher]

Fix some regressions on dijit widgets on mobile - Eric Durocher (IBM, CCLA)

[bill]

In [30808]:

Make Tree leverage dijit/a11yclick, which indirectly make openOnClick work for keyboard "click", fixes #16819 !strict. Also makes Tree more responsive on mobile, refs #15878.

Builds on work from [30806] and [30807].

[bill]

In [30818]:

Leverage ondijitclick in Calendar, to get the normalized touch handling in dojo/touch plus the keyboard click handling in dijit/a11yclick. Refs #15878 !strict.

[bill]

Changing title since this is mainly a ticket about changes to dojo/touch now (although it does fix the ScrollableView problem).