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

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

[bill]

In [30819]:

Leverage dojo/touch in CheckBox and MultiSelect, to get the normalized touch click handling. These two widgets are special because they are based on native form controls (<input type=checkbox> and <select multiple>), so they need to get a real click event rather than using a synthetic dojoclick type event. Luckily (for this case) this is how dojo/touch is currently working, by emitting a click event. Refs #15878 !strict.

[bill]

In [30820]:

Use ondijitclick in AccordionContainer, to get the normalized touch click handling. The keyboard click support will never be used in this case, since you can't focus the title of a closed accordion pane, but using ondijitclick as a way to trigger the touch normalization in dojo/touch. Refs #15878 !strict.

[bill]

In [30821]:

Use ondijitclick in ScrollingTabController, to get the normalized touch click handling on the left/right/menu buttons. The keyboard click support will never be used in this case, since you can't focus those buttons, but using ondijitclick as a way to trigger the touch normalization in dojo/touch. Refs #15878 !strict.

[bill]

In [30822]:

Use ondijitclick in TitlePane, to get the normalized touch click handling and keyboard "click" handling. Refs #15878 !strict.

[bill]

In [30827]:

Use touch events in _ListMouseMixin, for more responsive click handling. Refs #15878 !strict.

[bill]

In [30828]:

Suppress mousedown and mouseup events that occur 300ms after the synthetic click event is fired, since they may confuse client code. Also, do a better job of suppressing the native click event that (may) occur 300ms after the synthetic one; it sometimes appears on the DOMNode behind a popup that was closed in the 300ms interval. Patch from Eric Durocher (IBM, CCLA).

Refs #15878 !strict.

[bill]

In [30829]:

Use touch events in StackContainer, for more responsive click handling for TabContainer tabs. Refs #15878 !strict. Note that [30821] only fixed the left/right/menu buttons. This fixes the main tab buttons.

[bill]

In [30858]:

Don't call preventDefault() on the native click event, otherwise on Android text fields cannot be edited anymore. Not sure why, since we are generating a synthetic click event. Similarly, not sure why this doesn't break (native) checkboxes. Seems like they would be getting two clicks now instead of one. But, everything seems to work with this change. Refs #15878 !strict.

[bill]

In [30880]:

set -webkit-tap-highlight color to avoid late flash on iOS, fixes #16863, refs #15878

[Eric Durocher]

Synthetic clicks are sent even on disabled elements, patch attached to #16866 : ​http://bugs.dojotoolkit.org/attachment/ticket/16866/touch_click_disabled.patch

[bill]

In [30902]:

Don't generate click events due to touchstart/touchend on disabled <button> nodes. Fixes #16866, refs #15878 !strict. Also renamed "clicked" to "clickTracker".

[Eric Durocher]

Backtrack use of dojoClick to replace synthetic clicks in mobile - Eric Durocher (IBM, CCLA)

[bill]

Having more problems with this approach of generating faux click events, so will try another approach of just generating synthetic events with a different name.

Eric reports that: In dojox/mobile/tests/test_FormControls.html : if I click *exactly* on a RadioButton's check box it works as expected, but if I click on the label, other radios are not deselected, apparently because the widget does not receive the click...

From me: On iPad in dojox/mobile/tests/test_FormControls.html I'm seeing the checkbox toggle instantly (as we want) but then toggling again 300ms later, back to the original value. By my debugging, apparently what's happening is:

1. the user clicks <label for="cbox">Click me</label>
2. we emit synth click on that <label>
3. the click on <label> generates a click event on the associated checkbox <input type="checkbox" ...>. we sort-of block that native event, but not really, due to [30858], so the checkbox toggles.
4. 300ms later, the real click event on the <label> comes. again we try to suppress it, but we don't, due to [30858], so the checkbox gets another click event, and toggles again.

If I revert [30858] then clicking the label just doesn't do anything.

It seems like we are between a rock and a hard place... [30858] was needed on android because the synthetic click event wouldn't perform the browser default action. But as I worried, it causes repercussions [on iOS]. The other complication is of course that it's hard to suppress the click event from the touchstart/touchend without accidentally suppressing other click events.

[bill]

Also, the stopNativeEvents() code from [30828] stops phantom click events from appearing on items behind a Menu (that is closed by the click event). However, it's problematic for clicking small things. On dijit/tests/form/mobile.html (and other tests) clicking on the checkboxes or their labels often generates touchstart and touchend events on nearby nodes, such as the containing <fieldset> or it's <legend>. The checkbox or label still gets a click event though. Somehow the browser adjusts for it.

[bill]

In [30932]:

Rollback change to MultiSelect in [30819]. MulitSelect is a native form control that opens a popup, and there's no delay on click, so using dojoClick doesn't have any effect. Refs #15878 !strict.

[bill]

In [30936]:

Call evt.preventDefault() on native click event except for INPUT controls. This prevents the double click problem while still allowing keyboard to popup on android. Also, forward clicks on labels to be clicks on the associated nodes. Patch from Eric Durocher (IBM, CCLA). Refs #15878 !strict.

Also removing "target" property to on.emit() hash, since it's already specified as the first argument to on.emit().

Finally, add CSS to avoid flash when clicking Tree.

[bill]

In [30949]:

Don't call evt.preventDefault() for <textarea>, same as <input type=text>, refs #15878 !strict.

[bill]

In [30958]:

fix regression where mouse clicks not getting through (on devices that support both mouse and touch), fixes #15878 !strict

[cjolif]

In [31156]:

refs #15878. Fix two problems with synthetic clicks: virtual keyboard not hidden when clicking background + stopImmediatePropagation does not imply stopPropagation on Android 2.x. Thanks Eric Durocher & Sebastien Brunot(IBM, CCLA) (with review from Bill). !strict.

[bill]

In [31162]:

fix dijit/a11yclick paths to be relative, refs #15878

[Eric Durocher]

In [31244]:

refs #15878. Let applications set win.doc.dojoClick to false, and generate synthetic clicks like in 1.8 in that case. !strict

[Eric Durocher]

In [31323]:

refs #15878. Fix regression in mobile test case using native radio buttons.

[bill]

In [31403]:

remove no-longer used dependency after [30808], refs #15878 !strict

[Eric Durocher]

Coming back to ticket:15878#comment:5 and the specific problem on Galaxy Note 2: just tested with 1.9, all works OK now on that device. It is true, though, that if we set document.dojoClick = false; to disable the new 1.9 click machinery, Switch widgets for example do not receive click events, so the legacy code that generates click conditionally on some devices seems to fail on Galaxy Note 2. I don't think it is worth fixing as long as the default (new) dojoClick code works fine.

[Bill Keese <[email protected]…>]

In a776e1ec8117dccfce8c8ad88e571a50e89db82a/dojo:

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 f2d53fb8b0f0194521d226b2699dfe6b3a4c4b2a/dijit:

Error: Processor CommitTicketReference failed

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