[AccessFu] Introduce high-level touch gestures

RESOLVED FIXED in mozilla17

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: eeejay, Assigned: eeejay)

Tracking

Trunk
mozilla17
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
In B2G we need to handle touch gestures ourselves. So we need to figure out a way to translate individual DOM touch events into gestures that we could use, like double tap, swip, explore, etc.
(Assignee)

Comment 1

5 years ago
Created attachment 648945 [details] [diff] [review]
Introduce GestureMangler and add some basic gesture support.

This is still a WIP, not ready for review. Just putting it here for transparency and safe-keeping so it doesn't get lost over the weekend.

Comment 2

5 years ago
If it's of any help, we have Python code in NVDA to handle abstract finger touches and convert them into gestures. Obviously, it won't be of direct use, but the ideas might be useful. It's also GPL which might cause licensing issues, but I'm sure we can figure that out for Mozilla. :)
(Assignee)

Comment 3

5 years ago
(In reply to James Teh [:Jamie] from comment #2)
> If it's of any help, we have Python code in NVDA to handle abstract finger
> touches and convert them into gestures. Obviously, it won't be of direct
> use, but the ideas might be useful. It's also GPL which might cause
> licensing issues, but I'm sure we can figure that out for Mozilla. :)

Thanks Jamie! I think the patch I put here is fully functional. It was fun to implement from scratch. Just not ready to sign off on it.
(Assignee)

Comment 4

5 years ago
Created attachment 652197 [details] [diff] [review]
Introduce TouchAdapter and add some gesture support.

Still a bit raw, but I think it is a good start. Has everything we will mostly need.
Attachment #648945 - Attachment is obsolete: true
Attachment #652197 - Flags: review?(dbolter)
Comment on attachment 652197 [details] [diff] [review]
Introduce TouchAdapter and add some gesture support.

Review of attachment 652197 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/jsat/AccessFu.css
@@ +25,5 @@
> +  width: 100%;
> +  height: 100%;
> +  position: fixed;
> +  top: 0px;
> +  left: 0px;

Do you need to set a reasonable z-index to make sure it is always on top?
Is Mouse2Touch used?
(Assignee)

Comment 7

5 years ago
(In reply to David Bolter [:davidb] from comment #5)
> Comment on attachment 652197 [details] [diff] [review]
> Introduce TouchAdapter and add some gesture support.
> 
> Review of attachment 652197 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/jsat/AccessFu.css
> @@ +25,5 @@
> > +  width: 100%;
> > +  height: 100%;
> > +  position: fixed;
> > +  top: 0px;
> > +  left: 0px;
> 
> Do you need to set a reasonable z-index to make sure it is always on top?

Not really, since it is the last child appended.
(Assignee)

Comment 8

5 years ago
(In reply to David Bolter [:davidb] from comment #6)
> Is Mouse2Touch used?

For desktop B2G use.
(In reply to Eitan Isaacson [:eeejay] from comment #8)
> (In reply to David Bolter [:davidb] from comment #6)
> > Is Mouse2Touch used?
> 
> For desktop B2G use.

So hmmm we need a Utils.MozBuildApp switch for that I guess. Later is fine.
(Assignee)

Comment 10

5 years ago
Comment on attachment 652197 [details] [diff] [review]
Introduce TouchAdapter and add some gesture support.

Review of attachment 652197 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/jsat/TouchAdapter.jsm
@@ +62,5 @@
> +    this.glass.addEventListener('touchmove', this, true, true);
> +    this.glass.addEventListener('touchstart', this, true, true);
> +
> +    if (Utils.OS != 'Android')
> +      Mouse2Touch.attach(aWindow);

We check OS and not MozBuildApp because it will always be B2G, and we are interested to attach this when we are not on a device (ie. not in "Android".
(In reply to Eitan Isaacson [:eeejay] from comment #10)
> Comment on attachment 652197 [details] [diff] [review]
> Introduce TouchAdapter and add some gesture support.
> 
> Review of attachment 652197 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/jsat/TouchAdapter.jsm
> @@ +62,5 @@
> > +    this.glass.addEventListener('touchmove', this, true, true);
> > +    this.glass.addEventListener('touchstart', this, true, true);
> > +
> > +    if (Utils.OS != 'Android')
> > +      Mouse2Touch.attach(aWindow);
> 
> We check OS and not MozBuildApp because it will always be B2G, and we are
> interested to attach this when we are not on a device (ie. not in "Android".

Ah ok thanks! I had some kind of browser find-in-page fail when I went back to this review.
In the your naming usage of "Adapter", what is the semantic/ how are you using it? I always think of the Java idiom/pattern.
(Assignee)

Comment 13

5 years ago
(In reply to David Bolter [:davidb] from comment #12)
> In the your naming usage of "Adapter", what is the semantic/ how are you
> using it? I always think of the Java idiom/pattern.

I haven't really given it much thought. Just that we support two platforms and they each use a distinct "adapter" that translates different types of touch events in to higher level gesture events that VirtualCursorController listens for. Originally I called it a GestureMangler. I am open to more accurate names here.
OK - sorry for the delay BTW - my review session constantly interrupted.
Comment on attachment 652197 [details] [diff] [review]
Introduce TouchAdapter and add some gesture support.

Review of attachment 652197 [details] [diff] [review]:
-----------------------------------------------------------------

OK! r=me. Reviewing is hard. I hoep to revisit some parts of this in case I can think of improvements but don't block on this.

::: accessible/src/jsat/AccessFu.css
@@ +14,1 @@
>  #virtual-cursor-inset { 

Nit: trailing whitespace.

::: accessible/src/jsat/TouchAdapter.jsm
@@ +18,5 @@
> +const EXPLORE_THROTTLE = 100;
> +
> +var TouchAdapter = {
> +  // minimal swipe distance in inches
> +  SWIPE_MIN_DISTANCE: 0.4,

Did you derive these numbers or are they from some source?

@@ +30,5 @@
> +  // maximum consecutive
> +  MAX_CONSECUTIVE_GESTURE_DELAY: 400,
> +
> +  // delay before tap turns into dwell
> +  DWELL_MIN_TIME: 500,

Maybe call this DWELL_THRESHOLD?

@@ +236,5 @@
> +   * Compile a gesture from an individual touch point. This is used by the
> +   * TouchAdapter to compound multiple single gestures in to higher level
> +   * gestures.
> +   */
> +  compile: function TouchPoint_compile(aTime) {

Not sure about the name 'compile' here but will suffice and I'll suggest a different name later if I think of a better one.

@@ +249,5 @@
> +        return {type: 'tap', x: this.startX, y: this.startY};
> +      } else if (!this.done && duration == TouchAdapter.DWELL_MIN_TIME) {
> +        return {type: 'dwell', x: this.startX, y: this.startY};
> +      }
> +

Nit: needless line spacer.

@@ +276,5 @@
> +        else
> +          swipeGesture.type = 'swipeup';
> +      } else {
> +        // A perfect 45 degree swipe?? Not in our book.
> +          return null;

I think I'd make one of the former conditions into a conditionless else and fold a perfect 45 into one of them but your call. Actually maybe we'll want more deadspace in the 45 degree range later - dunno. What you have is fine to start with.
Attachment #652197 - Flags: review?(dbolter) → review+
(Assignee)

Comment 16

5 years ago
(In reply to David Bolter [:davidb] from comment #15)
> Comment on attachment 652197 [details] [diff] [review]
> Introduce TouchAdapter and add some gesture support.
> 
> Review of attachment 652197 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> OK! r=me. Reviewing is hard. I hoep to revisit some parts of this in case I
> can think of improvements but don't block on this.
> 

Yeah, this isn't going to be perfect the first time.

> ::: accessible/src/jsat/AccessFu.css
> @@ +14,1 @@
> >  #virtual-cursor-inset { 
> 
> Nit: trailing whitespace.
> 
> ::: accessible/src/jsat/TouchAdapter.jsm
> @@ +18,5 @@
> > +const EXPLORE_THROTTLE = 100;
> > +
> > +var TouchAdapter = {
> > +  // minimal swipe distance in inches
> > +  SWIPE_MIN_DISTANCE: 0.4,
> 
> Did you derive these numbers or are they from some source?
> 

I took them from my early extension prototype from last year. I think I made them up.

> @@ +30,5 @@
> > +  // maximum consecutive
> > +  MAX_CONSECUTIVE_GESTURE_DELAY: 400,
> > +
> > +  // delay before tap turns into dwell
> > +  DWELL_MIN_TIME: 500,
> 
> Maybe call this DWELL_THRESHOLD?

Sure.

> 
> @@ +236,5 @@
> > +   * Compile a gesture from an individual touch point. This is used by the
> > +   * TouchAdapter to compound multiple single gestures in to higher level
> > +   * gestures.
> > +   */
> > +  compile: function TouchPoint_compile(aTime) {
> 
> Not sure about the name 'compile' here but will suffice and I'll suggest a
> different name later if I think of a better one.
> 

Good!

> @@ +249,5 @@
> > +        return {type: 'tap', x: this.startX, y: this.startY};
> > +      } else if (!this.done && duration == TouchAdapter.DWELL_MIN_TIME) {
> > +        return {type: 'dwell', x: this.startX, y: this.startY};
> > +      }
> > +
> 
> Nit: needless line spacer.
> 

Yup.

> @@ +276,5 @@
> > +        else
> > +          swipeGesture.type = 'swipeup';
> > +      } else {
> > +        // A perfect 45 degree swipe?? Not in our book.
> > +          return null;
> 
> I think I'd make one of the former conditions into a conditionless else and
> fold a perfect 45 into one of them but your call. Actually maybe we'll want
> more deadspace in the 45 degree range later - dunno. What you have is fine
> to start with.

OK.
(Assignee)

Comment 17

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e24233c9339
Assignee: nobody → eitan
(Assignee)

Comment 18

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d9fbfd32c10
https://hg.mozilla.org/mozilla-central/rev/2e24233c9339
https://hg.mozilla.org/mozilla-central/rev/0d9fbfd32c10
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.