p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

[BUG] Firefox TouchStarted not working

Open IcyTv opened this issue 5 years ago • 16 comments

Most appropriate sub-area of p5.js?

  • [ ] Color
  • [ ] Core/Environment/Rendering
  • [ ] Data
  • [ ] Dom
  • [ ] Image
  • [ ] IO
  • [ ] Events
  • [ ] Math
  • [ ] Typography
  • [ ] Utilities
  • [ ] WebGL
  • [X] Other (Touch? User Interaction?)

Details about the bug:

TouchStarted is not registering in Firefox

  • p5.js version: latest? also web editor and react-p5 and therefore (devDependency) 1.0.0?
  • Web browser and version: Firefox! (77.0.1 - latest) but not on chrome...
  • Operating System: Windows, (not android - tested)
  • Steps to reproduce this: 2 Ways I tested:
    • The hard way: With react and react-p5
    • The easy way: on the p5 editor in a new sketch type:
      function setup() {
        createCanvas(400, 400);
       }
    
      function draw() {
        background(220);
      }
    
      function touchStarted() {
         console.log("Touch started"); 
      }
    
    I expected it to log "Touch started" but it does not do that. (mousePressed however works)

IcyTv avatar Jun 23 '20 22:06 IcyTv

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, be sure to follow the issue template if you haven't already.

welcome[bot] avatar Jun 23 '20 22:06 welcome[bot]

Just to add some more to this:

  • it is also reproducible on Firefox on macOS
  • touchEnded does work in the same sketch 🤔

dhoizner avatar Jun 24 '20 15:06 dhoizner

@outofambit @lmccart i saw that you added a userAgent filter for safari in onmousedown here: https://github.com/processing/p5.js/commit/e00e1508a61631be3485a7b0ca15ebeebb6312dc#diff-28fb3f6087a4c536f91f1986c0787a0fR640

this filter doesn't exist in onmouseup and if removed from onmousedown, touchStarted starts working in firefox.

i'm also not seeing the event duplication on any desktop browsers with the filter removed. i can open a pr removing it but wanted to get your thoughts first.

dhoizner avatar Jun 24 '20 16:06 dhoizner

Ah this is the cause of a problem with something that I was working on. Thanks for pointing it out! I spent a lot of time assuming that touchStarted should be working across devices. Le sigh! I don't know if those navigator.userAgent checks make sense anymore. I just looked at the property and 'Safari' is included in latest Firefox for Windows 10 but not on latest Firefox with Mac 10.15.5.

I tested extensively both with and without the navigator.userAgent checks pointed to by @dhoizner. All tests done with current main branch:

Mac 10.15.5

  • Chrome 83 works currently, works with filter removed
  • Firefox 77.0.1 does not work currently, works with filter removed
  • Safari 13.1.1 works currently, works with filter removed

Windows 10 v1903

  • Chrome 83.0.4103 works currently, works with filter removed
  • Firefox 77.0.1 does not work currently, works with filter removed
  • Edge 44.18362 works currently, works with filter removed

Android 9

  • Chrome 81.0.4044 event fires twice currently, event fires twice with filter removed

iOS 13.5.1

  • Chrome 83.0.4103 event fires twice currently, event fires twice with filter removed
  • latest Firefox works currently, works with filter removed

The navigator.userAgent check does break in more browsers but I think that it is apparent that we need a better, more thoroughly tested method for deciding when to fire these events.

stalgiag avatar Jun 25 '20 17:06 stalgiag

If anyone has ideas for a possible solution, I can test it on my temporary abundance of devices. My instinct is to say that that navigator.userAgent check should be replaced with something that only catches mobile Chrome.

stalgiag avatar Jun 25 '20 17:06 stalgiag

@stalgiag i’m relatively new here, so not sure whether this has been looked at before... are pointer events worth looking into? We could probably still provide mouse vs touch by using the pointerType.

dhoizner avatar Jun 25 '20 17:06 dhoizner

Welcome @dhoizner ! I don't have much experience with the input events part of the library. I know that one difficulty is that we want touchStarted and mousePressed to work interchangeably across devices that do or not have touch inputs so I am not sure that a pointerType check would work. I am not sure though! Maybe @lmccart or @outofambit have thoughts?

Here is the most recent relevant issue that I can find.

stalgiag avatar Jun 25 '20 18:06 stalgiag

Thanks! I’ve been digging around for issues that I could tackle:).

If we want them to work interchangeably, that might make pointer events even more compelling! I think this was the exact problem they were meant to solve.

With regards to this problem, it seems that things stay as broken without the check as they do with? Might be a good idea to remove the check in that case.

dhoizner avatar Jun 25 '20 19:06 dhoizner

Ah I see! I dug around the pointer events docs a bit more and this does look ideal but it lacks safari support. :-(

stalgiag avatar Jun 25 '20 19:06 stalgiag

Ah I see! I dug around the pointer events docs a bit more and this does look ideal but it lacks safari support. :-(

weird! Can i use... says that newer versions of Safari do support it, I wonder what the mismatch is.

dhoizner avatar Jun 25 '20 19:06 dhoizner

Ah I see! I dug around the pointer events docs a bit more and this does look ideal but it lacks safari support. :-(

weird! Can i use... says that newer versions of Safari do support it, I wonder what the mismatch is.

Looks like MDN needs to be updated; I'll report it. I just tested and works in Safari 13 (also in release notes). Safari 13 is supported for most iOS/macOS devices.

I don't know what the p5 community opinion is on only supporting newer versions of Safari. What do others think?

stalgiag avatar Jun 25 '20 19:06 stalgiag

Safari 13 is still too new to be the minimum version to be supported, at least not until Safari 14 is released (probably with the new macOS in the Fall) otherwise the official support of last two major versions would be a bit of a problem here.

Practically it probably won't cause a big problem if we use pointer events as most Apple users seems to be relatively up to date in terms of software updates.

limzykenneth avatar Jun 25 '20 20:06 limzykenneth

thoughts on how to proceed here? i can open up a pr to remove the filter- that would get firefox working and leave everything else the same.

in the meantime, i can also start on a pointer events implementation and we can figure out how that needs to interplay with mouse/touch events until safari 14 comes out.

dhoizner avatar Jun 27 '20 06:06 dhoizner

hey all thanks for digging into this. I agree, let's move to pointerEvents as soon as we can. looks like that's not yet, but @dhoizner if you want to start work on a PR for that so we have it ready to go that's great.

regarding firefox, I'm a little confused. are you saying firefox doesn't have navigator.userAgent? it seems based on caniuse.com that the last two version should have it. in any case, we could add a safety check because it looks like not all browsers do have it. rather than removing completely, I would suggest replacing with:

if (navigator.userAgent && 
    navigator.userAgent[0].toLowerCase().includes('safari') &&
    // etc) {

does that address the error you found?

lmccart avatar Jun 28 '20 16:06 lmccart

thanks everyone for digging into this! @lmccart i think calling navigator.userAgent itself works fine on firefox. my hypothesis is that either firefox has always had this behavior and we didn't notice or they changed their implementation of when touchStarted is called such that it needs the same workaround as safari. and after testing in macOS chrome, it looks like mousePressed is not fired at all by the browser itself, which i think explains why removing the userAgent check doesn't break it's behavior, as @stalgiag found.

(try this example in chrome: https://editor.p5js.org/outofambit/sketches/39neh8Vro. you'll see that mousePressed is never called by a mouse click, but touchStarted is even when i've defined both handlers myself.)

TLDR: these event behaviors are a mess in browsers today and i support moving the Pointer events API. i think we could do that now if we polyfill the pointer api for older browsers with https://github.com/jquery/PEP. (found this via modernizr.) the minified version of PEP is 24KB. thoughts?

in the shorter term, i think removing the userAgent check from mousePressed and touchStarted is fine, but as @stalgiag showed, it doesn't get us the consistent behavior we need across devices and browsers.

outofambit avatar Jul 03 '20 21:07 outofambit

I agree with @outofambit's TLDR above!

lmccart avatar Jul 04 '20 00:07 lmccart