Closed Bug 1182311 Opened 6 years ago Closed 6 years ago

[AccessFu] 3 finger triple tap is not reliable

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
feature-b2g 2.2r+
Tracking Status
firefox44 --- fixed
b2g-v2.2r --- fixed

People

(Reporter: eeejay, Assigned: eeejay)

References

Details

Attachments

(1 file, 1 obsolete file)

Getting a 3 finger triple tap is very hard.

This has something to do with the fact that we continue to track pointer identifiers after pointerup, and the next pointerdown. The identifier will not remain consistent. In other words, pointer "0" will be the first finger to touch the screen, and it won't always be the same finger each time, so you get an erroneous "travel".
Assignee: nobody → eitan
Attachment #8634162 - Flags: review?(yzenevich)
Blocks: 1176292
No longer blocks: 1176292
Blocks: 1176292
feature-b2g: --- → 2.2r+
Comment on attachment 8634162 [details] [diff] [review]
Make 3 finger triple tap more reliable.

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

Left some comments. Let me know what you think?

::: accessible/jsat/Gestures.jsm
@@ +415,5 @@
>      let complete;
>      let lastEvent;
>      for (let point of aPoints) {
>        let identifier = point.identifier;
>        let gesturePoint = this.points[identifier];

I think ideally we need to have a function that can lookup the existing gesturePoint, instead of overriding the one under index. We are loosing a lot of information like distance travelled etc, that might give us side effects. 

I would suggest instead of let gesturePoint = this.points[identifier]; have some sort of utility
for example this.findPoint; that would first check the identifier and then if there's one, verify that it is indeed the same point (within the threshold), if not, we would need to lookup the point with other id, but close enough coordinates. What do you think? Performance?

@@ +417,5 @@
>      for (let point of aPoints) {
>        let identifier = point.identifier;
>        let gesturePoint = this.points[identifier];
>        if (gesturePoint) {
> +        if (aType === 'pointerdown') {

I think this also needs to check for aCanCreate.

::: accessible/tests/mochitest/jsat/gestures.json
@@ +316,5 @@
> +      {"points": [{"identifier": 0, "x": 2.114583, "y": 1.572917}], "type": "pointerup"},
> +      {"points": [{"identifier": 0, "x": 1.4375, "y": 2.59375}], "type": "pointerdown"},
> +      {"points": [{"identifier": 1, "x": 1.083333, "y": 3.71875}], "type": "pointerdown"},
> +      {"points": [{"identifier": 2, "x": 2.15625, "y": 1.489583}], "type": "pointerdown"},
> +      {"points": [{"identifier": 0, "x": 1.4375, "y": 2.59375}, {"identifier": 2, "x": 2.15625, "y": 1.489583}], "type": "pointermove"},

Nit: here and below line length.
Comment on attachment 8634162 [details] [diff] [review]
Make 3 finger triple tap more reliable.

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

::: accessible/jsat/Gestures.jsm
@@ +415,5 @@
>      let complete;
>      let lastEvent;
>      for (let point of aPoints) {
>        let identifier = point.identifier;
>        let gesturePoint = this.points[identifier];

I think ideally we need to have a function that can lookup the existing gesturePoint, instead of overriding the one under index. We are loosing a lot of information like distance travelled etc, that might give us side effects. 

I would suggest instead of let gesturePoint = this.points[identifier]; have some sort of utility
for example this.findPoint; that would first check the identifier and then if there's one, verify that it is indeed the same point (within the threshold), if not, we would need to lookup the point with other id, but close enough coordinates. What do you think? Performance?

@@ +417,5 @@
>      for (let point of aPoints) {
>        let identifier = point.identifier;
>        let gesturePoint = this.points[identifier];
>        if (gesturePoint) {
> +        if (aType === 'pointerdown') {

I think this also needs to check for aCanCreate.

::: accessible/tests/mochitest/jsat/gestures.json
@@ +316,5 @@
> +      {"points": [{"identifier": 0, "x": 2.114583, "y": 1.572917}], "type": "pointerup"},
> +      {"points": [{"identifier": 0, "x": 1.4375, "y": 2.59375}], "type": "pointerdown"},
> +      {"points": [{"identifier": 1, "x": 1.083333, "y": 3.71875}], "type": "pointerdown"},
> +      {"points": [{"identifier": 2, "x": 2.15625, "y": 1.489583}], "type": "pointerdown"},
> +      {"points": [{"identifier": 0, "x": 1.4375, "y": 2.59375}, {"identifier": 2, "x": 2.15625, "y": 1.489583}], "type": "pointermove"},

Nit: here and below line length.
Attachment #8634162 - Flags: review?(yzenevich)
Attachment #8634162 - Attachment is obsolete: true
Attachment #8666894 - Flags: review?(yzenevich)
Comment on attachment 8666894 [details] [diff] [review]
Make 3 finger triple tap more reliable.

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

thanks!
Attachment #8666894 - Flags: review?(yzenevich) → review+
https://hg.mozilla.org/mozilla-central/rev/9bedea6a50d1
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.