After some investigation, and @JimishF pointing out that this is unrelated to #1864 and #1868, here's a whole new issue: select elements created using createSelect() are unresponsive on mobile browsers. I've tested this on:
To replicate this in Chrome or Safari:
$ python -m SimpleHTTPServer works for me on OSX.You should get no response from the select, but you can add elements to it using the button (just proving that it really is unrelated to #1864 and #1868)
The sketch:
var responseDiv, opts, addButton;
function setup() {
console.log('starting');
opts = createSelect();
opts.position(10, 10);
opts.size(150, 30)
opts.option('Pick a device', random(34));
opts.option('fee', random(34));
opts.option('fie', random(34));
opts.option('fo', random(34));
opts.changed(fum);
responseDiv = createDiv('tap the scan button to begin');
responseDiv.position(10, 100);
addButton = createButton('add an option');
addButton.position(10, 40);
addButton.touchEnded(addSomething);
}
function fum(event) {
console.log(event.target);
responseDiv.html(opts.value());
}
function addSomething() {
var number = random(34);
opts.option('another'+ number, number);
}
To test in PhoneGap:
To test in Chrome on OSX:
Strangely, I was able to get
I'm cross-posting this on Jiashan's p5Mobile repo just in case it belongs there instead, but it seems general enough that it's worth solving for p5 in general.
Thanks for the detailed report, and thanks @JimishF for helping disambiguate! I'll look into this as soon as I can and see what's up.
I think this is related to the fix for events firing twice (https://github.com/processing/p5.js/pull/1820#pullrequestreview-30677571). I believe the solution is to do something along the lines to what is suggested in this comment.
Commenting out this else statement (which are lines 19585-19588 in p5.js 0.5.8, if I'm translating to the current distribution correctly) does fix the problem with select tags, but it also causes touchEnd() events to fire twice. Tested on PhoneGap, iOS, and Nexus 5.
Here's a hack that seems to fix it for select tags. Replace the else block on line 19585 with this:
else {
if (e.target.nodeName !== 'SELECT') {
e.preventDefault();
}
return false;
}
Tested on the same targets as above. This fix does not re-introduce the double-firing of button touchEnd() events.
Tried to submit a PR, but git told me I didn't have authorization. It's lines 237 - 242 in /src/events/touch.js, hope that is enough info.
Thanks for testing this! I will have some time later this week to look at it and merge in the patch.
Should be good with the latest patch. Please see this note.
I'll give it a test to confirm later on today, thanks. You planning a 0.5.9 release anytime soon?
Works great on Android Chrome and PhoneGap.
Sorry to say, I take that back. I'm getting the double event error from button presses in Phonegap again. Not in Chrome on OSX though. Will do more testing...
Confirmed. Button press happens twice every time with every touchStarted() and touchEnded(). Select items work normally and fire only one event on every changed(). Same testing method as above.
agh i thought i'd finally got it! are you seeing this behavior on mobile without phonegap? i wonder if something is going on behind the scenes there that i haven't accounted for..
I am not seeing the issue on android chrome -- will test on phone gap later today
When I test using the Chrome JS console and toggle the device toolbar to simulate various Anrdoid or iOS devices, I don't see it. When I run it in Chrome on an actual android device, I don't. (tested by putting a counter in the callback for the button, incrementing it when the function's called, and putting it in a div) But when I run it in a cordova app on a phone, I do. Which suggests that the browser shell for Cordova/PhoneGap is handling the events differently than Chrome on Android, at least.
Aha, my best guess right now is that these if (!window.PointerEvent) checks I added that prevent the duplicate firing may not be returning the correct result on phonegap because of the way the browser is wrapped. I can take a look at this tomorrow, or @mlarghydracept if you are looking into this later today, that is where I'd start (you can also check out this comment for more background).
@tigoe I am still having trouble recreating the issue. Reading your responses it seems like the latest problem is a double firing with touchStarted & touchEnded. Please correct me if I am wrong but based on that assumption I have a simple sketch that I believe to be close to your test code --
var addButton;
var currentNumber = 0;
function setup() {
createCanvas(windowWidth, windowHeight);
addButton = createButton('CLICK ME');
addButton.position(200, 400);
addButton.touchEnded(increment);
}
function draw() {
background(255);
textSize(64);
text(currentNumber, 200, 300);
}
function increment() {
currentNumber++;
}
With that sketch everything is working in all of my test environments. I built the sketch into a Cordova app which I ran on my Marshmallow Android phone and the number goes up once per press as expected.
Any ideas about differences between our test sketches or environments?
@mlarghydracept Try this:
Use the p5.js and p5.dom.js files from the latest build (after @lmccart's patch) and then use this sketch, same instructions as above:
var responseDiv, opts, addButton;
var counter = 0;
function setup() {
console.log('starting');
opts = createSelect();
opts.position(10, 10);
opts.size(150, 30)
opts.option('Pick a device', random(34));
opts.option('fee', random(34));
opts.option('fie', random(34));
opts.option('fo', random(34));
opts.changed(fum);
responseDiv = createDiv('tap the scan button to begin');
responseDiv.position(10, 100);
addButton = createButton('add an option');
addButton.position(10, 40);
addButton.touchEnded(addSomething);
}
function fum(event) {
console.log(event.target);
responseDiv.html(opts.value());
}
function addSomething() {
counter++;
console.log(counter);
responseDiv.html(counter);
var number = random(34);
opts.option('another'+ number, number);
}
Tap the button. You'll get a double tap about 90% of the time.
What I've found is that if I tap, then swipe my finger out of the button before I lift, I can consistently get a single tap. But if the touch ends in the same place it started (e.g. in the button), then counter goes up by 2.
I get the same results with your code.
FWIW, I am running Android 5.1 on a Huawei ALE-L04. I am compiling with Android 6.1.2, though, but I checked by compiling with 5.1 as well, with the same results.
I'm using Android Monitor to watch the console too, FWIW.
@tigoe This is tough. I definitely have the most recent build and I used your exact code but I am still unable to recreate the problem in a Cordova app running on/compiling with Android 6.0 on a LG G3. Below I have pasted a snippet of the log from Android Monitor which shows expected behavior. As you can see I tapped 53 times without a single double tap or double increment.
04-09 10:13:18.970 27772-27772/io.cordova.hellocordova I/ViewRootImpl: ViewRoot's Touch Event : ACTION_DOWN
04-09 10:13:19.641 27772-27772/io.cordova.hellocordova I/ViewRootImpl: ViewRoot's Touch Event : ACTION_UP
04-09 10:13:19.647 27772-27772/io.cordova.hellocordova I/chromium: [INFO:CONSOLE(28)] "51", source: file:///android_asset/www/sketch.js (28)
04-09 10:13:23.435 27772-27772/io.cordova.hellocordova I/ViewRootImpl: ViewRoot's Touch Event : ACTION_DOWN
04-09 10:13:24.410 27772-27772/io.cordova.hellocordova I/ViewRootImpl: ViewRoot's Touch Event : ACTION_UP
04-09 10:13:24.416 27772-27772/io.cordova.hellocordova I/chromium: [INFO:CONSOLE(28)] "52", source: file:///android_asset/www/sketch.js (28)
04-09 10:13:26.598 27772-27772/io.cordova.hellocordova I/ViewRootImpl: ViewRoot's Touch Event : ACTION_DOWN
04-09 10:13:26.684 27772-27772/io.cordova.hellocordova I/ViewRootImpl: ViewRoot's Touch Event : ACTION_UP
04-09 10:13:26.689 27772-27772/io.cordova.hellocordova I/chromium: [INFO:CONSOLE(28)] "53", source: file:///android_asset/www/sketch.js (28)
I'll try to think of other possible issues and get back if any ideas come up. I will try testing on another phone if I get a chance.
@tigoe could you try two things for us? inside setup, can you place this line:
function setup() {
console.log("pointer event = "+window.PointerEvent);
...
}
and then at the end of your sketch add a touchEnded function:
function touchEnded() {
console.log('touch ended');
}
I'm curious (1) whether your cordova app is registering that your browser supports window.PointerEvent, and (2) if this is a problem with the element-specific listener attached to the button, or with touches anywhere on the screen (they're handled in slightly different ways).
Thanks both. It is a hairy one.
Lauren, in response to your added lines, I'm getting "Pointer event = undefined" on startup and "touch ended" on every touchEnd. Following up on @mlarghydracept's idea, I tried it also on a Nexus 7 running android 5.1.1 and got the same results.
FWIW:
$ cordova -v
6.5.0
$ phonegap -v
6.4.8
Also worth noting that I upgraded my ADK tools a couple days ago too, so they are current.
After thinking through all this, it seems perhaps the best solution is just to remove the built-in p5 helper functionality that tries to fire mouse events for touch events. The new window.PointerEvent API does this now in some browsers, which is what's causing the problem. The previous patch tried to detect this and stop the duplicate event firing in this case, but we are having a hard time debugging why this is still not working in some cases.
In this proposed solution, we remove all duplicate firing of mouse/touch events and let the browsers/user handle it. While this adds a little work for the user in some cases, there will at least not be any double firing or lost touchEnd events. Thoughts?
Would that mean that touch events and mouse clicked events are separate, and the user needs to assign callbacks for both if they want things to be compatible? I imagine most browsers would handle that automatically by now, so it might be a good solution. Why not make a test branch and see how it works?
Picking this up to work on next week. Just checking that the desired behavior here is to remove the automatic linking of touch and mouse events (hoping that the various browsers now do this automatically)?
Can verify the double tap issue is still happens on my Nexus 5X with Android 8.1.0 and Chrome 67.0.3396.87.
@kjhollen I think that's the best solution. and to update the docs so it's clear what the expected behavior is.
now that I've dug into this a little bit鈥攖o clarify, the desired behavior is:
using mouse events will (by default on up-to-date mobile browsers) work without having p5 generate the mouse events via touch. removing code the existing code that does this prevents some duplication of events.
however, it also means that touch events will not fire on desktop browsers. so using @tigoe's example above, you'd need to change addButton.touchEnded(addSomething); to addButton.mouseReleased(addSomething); to work on both mobile and desktop. with my updates, the touchEnded version only works in mobile browsers or simulations of mobile devices via desktop Chrome dev tools. however, mouseReleased works for both mobile & desktop.
I think the code is ready, but there is a lot of documentation to read & check if updates are needed. I'll be working my way through that this week.
@kjhollen if I'm reading this right, the mouse behaviors should always work/fire on mouse or touch (on up-to-date browsers). the touch behaviors will only work on touch. is that correct?
@lmccart yes, that's right!
@kjhollen that sounds reasonable to me :)
in testing to update the documentation, I realized I had misunderstood something: we do need to keep forwarding touch -> mouse events to make the mouse events always work by default.
the mobile/touch browser does not generate corresponding mouse events for all touch events by default; however, it does ALWAYS generate a set of mouse events when a quick-tap occurs! so the real culprit here is p5.Element.touchEnded, which signs up for a mouseup event. It will get both touchended and mouseup callbacks when the user taps quickly, which is what usually happens with a button. however, a longer press does not generate a mouseup! so this is why it appears to work intermittently.
so, with that in mind, it is an option to keep forwarding mouse -> touch and touch -> mouse in the scope of p5 global functions. I think it makes sense to scaffold this for users working primarily in the global scope, and have found it handy for testing when I work with mobile apps.
for p5.Element, I think we can remove the event doubling because it is handled a different way鈥攁nd write documentation to instruct the user to sign up for multiple events, because this is a more advanced use case. does that make sense?
fixed with #3082