dojo/touch with dojoClick=true breaks interaction with "native" HTML input of type "checkbox" or "radio" on iOS 8 and Chrome / Android 4.4

[ysazak]

I use a regular html checkbox elemnt in my mobile application.

after call below code block, checkbox doesnt respond on mobile devices(Ipad Ios8, Android 4.4). Please check jsfiddle test case aswell.

        require(["dojox/mobile/parser", "dojox/mobile"],
			function (parser) {
			    parser.parse();
			});

​http://jsfiddle.net/ysazak/wpL0L5p4/6/

[dylan]

I fixed your fiddle to load async: ​http://jsfiddle.net/dylan/wpL0L5p4/7/

I notice that this example works on iOS7, but breaks on iOS8.

If this is a bug and not a misuse of dojox/mobile, it pretty much breaks things badly.

[Adrian Vasiliu]

Here is a more minimal version of the fiddle: ​http://jsfiddle.net/adrian_vasiliu/fwhg8dae/ Requiring any dojox/mobile module isn't necessary for reproducing the issue. In this variant of the fiddle, the only required module is "dojo/touch", plus setting document.dojoTouch at true (as dojox/mobile/common does - this feature of dojo/touch has been introduced for dojox/mobile).

There will be further investigation. For now, two solutions/workarounds:

1/ Use dojox/mobile/ComboBox instead of <input type="checkbox">. Besides avoiding this issue, it also provides a nicer look and feel. I've put it in ​http://jsfiddle.net/adrian_vasiliu/wxuyyrbb/ (notice that it also imports the CSS of dojox/mobile/ComboBox by importing dojox/mobile/deviceTheme.js as external resource).

2/ If you don't want to use dojox/mobile/ComboBox, just set document.dojoTouch at false after dojox/mobile/common loads. I've put it in ​http://jsfiddle.net/adrian_vasiliu/eqpr53ap/. The drawback is that there is a 300 ms delay between touching the checkbox and its state change. The use of dojox/mobile/ComboBox seems preferable to me.

[Adrian Vasiliu]

The issue is coming from this preventDefault() call: ​https://github.com/dojo/dojo/commit/33be784ac33a090cc9091ef449ef930064802f7d#diff-4d94e8ca70a3b3b428f55af8bcf13a34R122

[Adrian Vasiliu]

I've put a candidate solution in ​https://github.com/AdrianVasiliu/dojo/commit/34bbe042d1fb4a31659a78f0c4aaecd4a91ae620 (change in dojo/touch). A manual test case: ​https://github.com/AdrianVasiliu/dojox/commit/5fe1b982317673703369f2270ff741108e72a81c .

@bill / @edurocher, what would you think about it? Of course, it's not an ideal solution, but given the current dojo/touch I'm not sure we can do better.

If there's no disagreement on it, I'll further test and transform it in a pull request.

[bill]

I looked at this a long time. I guess it's OK. It's ugly, but solves the problem, and the code was already ugly before your change.

One alternate approach is for dojo/touch to never call preventDefault() for any <input> nodes, but to make dijit/form/Checkbox etc. call preventDefault() manually. Not sure if that is better though. The widget code would need to setup a listener in the capturing phase (addEventListener(..., true)) to get to the event even though dojo/touch calls stopImmediatePropagation().

[Eric Durocher]

The fix looks OK to me as it should be harmless for widgets. Maybe the condition could be made clearer, as it is becoming more and more difficult to read...

Also, if the behavior really changed with iOS8 and Android 4.4, is it still OK with older versions and the fix (which does not test OS versions)?

[Adrian Vasiliu]

I've also tested with iOS 7 and it looked... even better with the fix than without. I mean, strangely, without the fix on iOS 7.1.2 (and with dojo/touch and dojoClick=true) the native checkbox "sometimes" works, while with the fix it always works... Hence it doesn't seem so clear that the fix is only useful for iOS >= 8. Anyway, so from this test the fix seems rather benefical for both iOS 7 and 8.

I'll also test with iOS 6 (although low priority) and with older Android with stock browser and Chrome.

[ben hockey]

Replying to bill:

One alternate approach is for dojo/touch to never call preventDefault() for any <input> nodes, but to make dijit/form/Checkbox etc. call preventDefault() manually.

touch events are complicated and i'm not an expert with them but this option sounds better to me since code in the dojo namespace doesn't need to add special knowledge of something related to the dijit namespace - it puts the responsibility of the preventDefault() call with the code that depends on the call. i think it is worth pursuing this option and only considering adrian's suggestion if this option isn't possible.

[Adrian Vasiliu]

doesn't need to add special knowledge of something related to the dijit namespace

Just to mention that dojo/touch already had a similar solution (checking a dijit CSS class): ​https://github.com/dojo/dojo/blob/1.10/touch.js#L182

Of course, we may prefer not adding another similar check. But this proposed change is similar to the existing one, and reengineering dojo/touch entirely (plus readapting dijit and dojox/mobile to possible changes, and testing everything on all supported mobile browsers) to really clean it up is probably not the best thing we can afford doing now.

[dylan]

Given that Dijit depends on Dojo, but Dojo does not depend on Dijit, that's really not a good thing.

Just because it's there doesn't mean we should do it again. That should really be fixed/changed, not replicated. :)

[bill]

The advantage of Adrian's approach is that it's the least invasive, so the least likely to cause problems (side effects). This is an important consideration, especially for updates to patch releases (i.e. 1.10.3 and possible further backports).

I did think of another approach though, which is probably better than my original suggestion. When dojo/touch emits the dojoclick event, it could also programatically change the state of the <input type=checkbox>. Not sure if you can call node.click(), or you would have to do something manual like node.focus(); node.checked=true; on.emit(node, "change", ...) (since IIRC change events are only emitted for user instantiated changes. In any case, the point is that you could keep calling preventDefault() on checkboxes, and you would get rid of the 300ms delay "problem" for native checkboxes too.

[dylan]

I guess we should just land the proposed fix for 1.11 then.

Adrian, do you have time to create a pull request?

[Adrian Vasiliu]

Yes, I can create the PR, if bill is still okay with it. FYI I checked whether the issue is still there with iOS 9: yes, still there, and the fix still fixes it. Tested on iOS 9.0.2 on iPad Air 2. I'll also check Android 5 and 6.

[bill]

Sure, sounds fine.

[Adrian Vasiliu]

Okay, hope to find a bit of time for it soon.

[beglee]

We've been affected by this issue in our product, which currently uses Dojo v1.9.7. Will the fix be backported to older versions?

[Adrian Vasiliu]

Yes, at least in my eyes the fix would be backported to 1.10 and 1.9.

By the way, I did spend some time working on preparing the PR, mostly fighting to find a way to get the functional tests running such that I can add one for this issue. So far wasn't successful (on my Windows machine), either locally or remotely on saucelabs, but I keep working on it.

[Adrian Vasiliu]

And I said I'll check Android. This is done. The issue hurts on Android 4.4.4, 5.1 et 6.0 (all tests on Nexus 5 physical device) just as on iOS 8 and 9, and the proposed fix does fix it for all these Android versions too.

[Adrian Vasiliu]

Just an update, running the functional tests works for me on a Mac (while I wasn't able to get them running on my PC).

[beglee]

I've tried out the proposed fix in 1.9.7 in our test environment, and it looks good for the most part. However, when I click on the LABEL associated with a checkbox/radio, the preventDefault still kicks in. Would there be a downside to adding [e.target.tagName != "LABEL"] to the if statement?

[Adrian Vasiliu]

I don't reproduce with Safari / iOS 9.0.2 / iPad Air 2. With an HTML such as

<input type="checkbox" id="test"><label for="test">Test</label>

I do get the state of the checkbox changing when touching the "Test" label, as long as I use the proposed fix.

If you use id=test instead id="test", please add the missing double-quotes. Also, on my iPad with this iOS version, I need to clear Safari's cache and even to reload the page a number of times before seeing the effect of changes in the JS code... This might be a source of confusion.

If you still reproduce despite the change mentioned above, please tell the exact device and OS version.

[Adrian Vasiliu]

PR: ​https://github.com/dojo/dojo/pull/173

[Adrian Vasiliu]

Fix merged into master via ​https://github.com/dojo/dojo/commit/961860c16a70204097718f0d65d0fc1afafcfb40. Backport to 1.9 and 1.10 will be done once we get enough confidence from users' feedback for master/1.11.