Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#16649 closed defect (fixed)

dojo/on touch event bug Firefox Android

Reported by: Wouter Hager Owned by: Kris Zyp
Priority: undecided Milestone: 1.8.4
Component: Events Version: 1.8.3
Keywords: Cc: Armin Pfurtscheller
Blocked By: Blocking:

Description

The event dojo/on returns in Firefox 18 Android is buggy. Lots of properties don't even show up in the debugger. The main problem is that there is no target in the event. I suppose the native event is not properly handled.

I know dojo doesn't currently claim to support Firefox on Android, but please keep this ticket open for enhancements. I'm still submitting a bug, because, well, it is.

Attachments (2)

on.patch (1.4 KB) - added by Wouter Hager 6 years ago.
quick&dirty patch
on.alternate.patch (545 bytes) - added by Luis Montes 6 years ago.
another option to fix this

Download all attachments as: .zip

Change History (22)

comment:1 Changed 6 years ago by bill

Component: GeneralEvents
Owner: set to Wouter Hager
Status: newpending

This ticket is rather vague, what type of event are you talking about? A click event?

comment:2 Changed 6 years ago by Wouter Hager

Status: pendingnew

A touch event. It's in the description.

I found the problem to be in the fixTouchListener function. The original event is immutable, but also calling it as a constructor for new Event doesn't work. I don't know why that is.

The quick fix would be to not construct a new event, but to clone originalEvent:

var event = {};
for(var name in originalEvent) event[name] = originalEvent[name];

Works for me.

Changed 6 years ago by Wouter Hager

Attachment: on.patch added

quick&dirty patch

comment:3 Changed 6 years ago by bill

OK, your summary says "touch event", didn't notice that at first, although it doesn't say which one (touchstart, touchmove, touchend, etc., or even one of the synthetic dojo/touch events). Perhaps all of them are failing.

You might be able to use lang.delegate() too. I'm not sure why the code for older iOS is using that homegrown way to delegate.

comment:4 Changed 6 years ago by Wouter Hager

Yes, all touch related events fail, as far as I can tell.

I tried

var event = lang.delegate(originalEvent);

but then the properties cannot be read. The code fails @ changedTouches, which cannot be read. It's all very odd. The only way I can fix it currently is to use the quick&dirty hack.

comment:5 Changed 6 years ago by Wouter Hager

Or slightly more dojo:

var event = lang.mixin({},originalEvent);

will work too.

comment:6 Changed 6 years ago by Armin Pfurtscheller

hi there,

can confirm the bug on firefox for android (18.0.2). i am using the following setup:

on(node,touch.press,function(event){ ... });

according to http://dojotoolkit.org/reference-guide/1.8/dojo/touch.html 'press' is mapped to 'touchstart'. would be nice if you could my me on the 'cc' list to receive an update to this ticket - in the meantime the bugfix is working, thank you!

comment:7 Changed 6 years ago by dylan

Cc: Armin Pfurtscheller added

comment:8 Changed 6 years ago by bill

Owner: changed from Wouter Hager to Kris Zyp
Status: newassigned

Assigning to Kris since it's his code, although not sure if we want to add extra code to dojo to support this, or just wait for firefox to fix their bug.

comment:9 Changed 6 years ago by Luis Montes

Is there confirmation that mozilla considers this a bug? Firefox beta 19 on android still has this same behavior.

I don't see a related issue on https://bugzilla.mozilla.org

comment:10 Changed 6 years ago by Kris Zyp

I am fine with this patch, it is not really any more code than is currently in fixTouchListener. However, we do need to verify that it still works in the earliest iOS that we currently support (don't recall what that is), as that code branch was specifically for handling older iOS.

Changed 6 years ago by Luis Montes

Attachment: on.alternate.patch added

another option to fix this

comment:11 Changed 6 years ago by Luis Montes

Another possibility is to skip that section of the code for FF altogether.

I think this issue is kind of a big deal given that Firefox OS was released yesterday. Just skipping the section for FF might be the least intrusive way to go about it.

comment:12 Changed 6 years ago by phated

I think the easiest and safest way to get this working in firefox for mobile (FFOS and FF for Android) is monteslu's patch. Also, it shouldn't have any unexpected side-effects. Hopefully this lands soon.

comment:13 Changed 6 years ago by Luis Montes

might have to use the alternate patch.

Just tested first patch on iOS 6 and touch events weren't working.

comment:14 Changed 6 years ago by Adrian Vasiliu

Let me mention that we have dojox/mobile customers who complain about this issue. For some reasons they need to run on Firefox Android although it is not a browser officially supported by dojo/mobile. For their sake, getting a fix for this issue would be very helpful (in both 1.8.x and 1.9).

Now, I have tested demos/touch/demo.html with on.patch and on.alternate.patch using Firefox 19.0.2 on HTC One X Android 4.0.4. From this test, it seems the alternate patch is much less effective than the original patch for fixing the issues in Firefox Android, especially in trunk (vs. 1.8 branch). Details of the test results for demos/touch/demo.html:

1) 1.8 branch

a) as-is:

rotation touch gesture: non responsive; dragging slider knob: non responsive; taping slider: non responsive; "hold down to lock" button: non responsive

b) on.patch

rotation touch gesture: OK; dragging slider knob: OK; taping slider: OK; "hold down to lock": OK

c) on.alternate.patch

rotation touch gesture : responsive but only turns a little bit (still misbehaves); dragging slider knob: OK (but sometimes doesn't move); taping slider: OK; "hold down to lock": non responsive

2) trunk

a) as-is

rotation touch gesture: non responsive; slider: dragging knob: non responsive; slider: taping: non responsive; "hold down to lock" button: non responsive

b) on.patch

rotation touch gesture: OK; dragging slider knob: OK; taping slider: OK; "hold down to lock": OK

c) on.alternate.patch

rotation touch gesture: non-responsive; slider: dragging knob: non-responsive; slider: taping: OK; "hold down to lock": non responsive

From the above, it appears on.patch is much better than on.alternate.patch for Firefox Android, at least in demos/touch - but the same holds for the impact on dojox/mobile apps.

On the other side, I do reproduce monteslu's finding: demos/touch is broken by on.patch on iPhone 5 iOS 6.0.2 (nothing is touch-responsive).

Hence, to get it work fine everywhere, it would seem that we could use a variant of on.patch which conditionally uses the new code only if (has("ff") && has("android")), isn't it?

Last edited 6 years ago by Adrian Vasiliu (previous) (diff)

comment:15 Changed 6 years ago by Kris Zyp

What exactly is Firefox doing when we delegate from the original event? Is it throwing an error? Or ignoring property writes? Is there an error or behavior we can branch on?

comment:16 Changed 6 years ago by Adrian Vasiliu

No console in Firefox Android 19, but using the remote debugger (https://hacks.mozilla.org/2012/08/remote-debugging-on-firefox-for-android/) I see it breaks on error as soon as I touch the screen at var firstChangeTouch = event.changedTouches[0]; at the end of fixTouchListener in on.js.

The error:

NS_ERROR_XPC_BAD_OP_ON_WN_PROTO: Illegal operation on WrappedNative prototype object @ http://9.128.82.108:8020/dojo-trunk-pure/dojo/on.js:511 (the line number holds for the trunk version)

Searching with this error message I find

https://bugzilla.mozilla.org/show_bug.cgi?id=790303

which is apparently exactly the cause of our trouble (bug open by Colin Snoover in Sept. 2012).

Last edited 6 years ago by Adrian Vasiliu (previous) (diff)

comment:17 Changed 6 years ago by Kris Zyp

Resolution: fixed
Status: assignedclosed

In [30886]:

Copy properties for mobile FF, fixes #16649 !strict

comment:18 Changed 6 years ago by Kris Zyp

In [30887]:

Copy properties for mobile FF, backport, refs #16649 !strict

comment:19 Changed 6 years ago by Kris Zyp

Milestone: tbd1.8.4

comment:20 Changed 6 years ago by bill

#16941 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.