Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#14720 closed defect (worksforme)

[cla] dojo/on.js : fixTouchListener is undefined

Reported by: loodwig Owned by: Kris Zyp
Priority: high Milestone: tbd
Component: Events Version: 1.7.1
Keywords: Cc:
Blocked By: Blocking:

Description

in dojo/on.js, the addListener() function attempts to correct the listener if has("touch") is true. However, within the addListener() function is no reference to fixTouchListener(), and an error is thrown / execution is halted when the user agent is:

Mozilla/5.0 (Linux; U; Android 2.3.6; en-us; Nexus S Build/GRK39F) AppleWebKit?/533.1 (KHTML, like Gecko) Version/4.0 Mobile Safari/533.1

The fixTouchListener() function appears to be defined later in the file, but it is not in scope of its calling components at line 120 and line 127. To solve this issue, I recommend the function be brought within the scope of addListener(), preferably within the if(has("touch")){ block, so that it is defined when needed for mobile browsers that support touch.

I have attached a diff from 27684 (HEAD at the time of making this ticket), which shows my proposed solution.

Attachments (2)

on.js.diff (4.8 KB) - added by loodwig 7 years ago.
diff between 27684 and my suggested solution
on.js.reproduction (1.3 KB) - added by loodwig 7 years ago.
html and js needed to reproduce the problem

Download all attachments as: .zip

Change History (13)

Changed 7 years ago by loodwig

Attachment: on.js.diff added

diff between 27684 and my suggested solution

comment:1 Changed 7 years ago by Colin Snover

Keywords: needscla added
Owner: set to loodwig
Priority: undecidedhigh
Status: newpending

Have you signed a CLA or are you under a CCLA? If so, what is your name? If not, could you please do so at http://dojofoundation.org/about/cla? Thanks!

comment:2 in reply to:  1 Changed 7 years ago by loodwig

Status: pendingnew

Replying to csnover: CLA form is being faxed now.

Changed 7 years ago by loodwig

Attachment: on.js.reproduction added

html and js needed to reproduce the problem

comment:3 Changed 7 years ago by Colin Snover

Keywords: needscla removed
Status: newopen
Summary: dojo/on.js : fixTouchListener is undefined[cla] dojo/on.js : fixTouchListener is undefined

CLA received. Thank you!

comment:4 Changed 7 years ago by Colin Snover

Owner: changed from loodwig to Kris Zyp
Status: openassigned

Kris can you review/triage?

comment:5 Changed 7 years ago by bill

Component: GeneralEvents

comment:6 Changed 7 years ago by Kris Zyp

Resolution: worksforme
Status: assignedclosed

I don't follow this ticket. fixTouchListener is defined in the module's factory scope, which contains addListener and can be referenced by the addListener. This function is being successfully called on touch events, and touch events are being properly normalized by this function. I can't see anything happening wrong (or anything at all) in the attached test case.

comment:7 Changed 7 years ago by loodwig

You're absolutely right, I've only been able to get this to happen using the 1.7.1-src unzipped locally. If I use trunk, the cdn, or the download of 1.7.1 (built), this problem goes away. I'll continue to research, as the problem appears to only be on my end. Sorry for that.

comment:8 in reply to:  6 Changed 7 years ago by loodwig

Replying to kzyp: Hmm, it appears that with further testing I've ruled out my environment to the best of my knowledge. I've added the following site that demonstrates (using the attached code) the error consistently with the aforementioned android user agent. I'm noticing it 100% of the time using chrome set to the user agent as well as a android device. I figured a public site might help:

http://loodwig.org/14720/

comment:9 Changed 7 years ago by Colin Snover

That test case does not raise an error, just says “Loading…”. What is it supposed to do? What are the reproduction instructions?

comment:10 Changed 7 years ago by Kenneth G. Franqueiro

I've also attempted to reproduce an error locally using the attached reproduction material, running Android 2.3.7 with weinre loaded in the page, but I see no errors.

Based on what I heard discussed in the IRC channel earlier, it sounds like the error was only reproduced when UA-spoofing was used to attempt to test the page in an "Android-like" environment but from a desktop browser - in which case, any has("touch") code won't be executed, which in this case includes the code that both calls and defines the function in question.

Either way, I'm not sure how this error could be coming up... I agree with Kris' assessment.

comment:11 Changed 7 years ago by loodwig

I concur as well. I've tested on a few other android devices outside of my testing environment, and I am not getting the aforementioned error. Initially, I was getting a different but similar error (which may very well be with my code). At this point, I'll assume all errors I've received came from my "spoofing" of the user agent in order to create a test case to diagnose this separate error. If (and I emphasize if) there is an error with dojo, it's a separate issue. In the future, I'll do better testing before filing a ticket.

Note: See TracTickets for help on using tickets.