651554 - permaorange: browser_tapping.js | Fired a good double tap

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Phil Ringnalda (:philor)]

TEST-INFO | chrome://mochitests/content/browser/mobile/chrome/browser_tapping.js | Test good double tap
TEST-INFO | chrome://mochitests/content/browser/mobile/chrome/browser_tapping.js | ---- event check: failed match (TapDouble != TapSingle

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/mobile/chrome/browser_tapping.js | Fired a good double tap
TEST-INFO | chrome://mochitests/content/browser/mobile/chrome/browser_tapping.js | Test two single taps in different locations
TEST-PASS | chrome://mochitests/content/browser/mobile/chrome/browser_tapping.js | Fired two single taps in different places, not a double tap
TEST-INFO | chrome://mochitests/content/browser/mobile/chrome/browser_tapping.js | Test a pan - non-tap event
TEST-PASS | chrome://mochitests/content/browser/mobile/chrome/browser_tapping.js | Fired a pan which should be seen as a non event
TEST-INFO | chrome://mochitests/content/browser/mobile/chrome/browser_tapping.js | Test a long pan - non-tap event
TEST-PASS | chrome://mochitests/content/browser/mobile/chrome/browser_tapping.js | Fired a pan + delay which should be seen as a non-event
TEST-INFO | chrome://mochitests/content/browser/mobile/chrome/browser_tapping.js | Test a good long pan
failed to validate file when downloading /mnt/sdcard/tests/mochitest.log!
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/mobile/chrome/browser_tapping.js | Test timed out

Comment 2

[Phil Ringnalda (:philor)]

Why yes, yes I do know how to search before I file, I just don't ever actually remember to do it :)

However, that patch just fixes the timeout at the end, not the permaorange in the "Fired a good double tap" part.

Comment 3

[Phil Ringnalda (:philor)]

Not Android-specific, the n900s do the same thing.

Comment 4

[Wesley Johnston (:wesj)]

Attachment: Potential Patch

On desktop I occasionally see this. There is looks like more than two clicks are being fired. I have seen this happen before with local tabs. Events fired in the local tab can propagate back up and be received by input.js again.

So one potential solution to this problem (if it is what is causing the tests for fail) is to try and catch those events in input.js.

Comment 5

[Benjamin Stover (:stechz)]

Comment on attachment 527857 [details] [diff] [review]
Potential Patch

That input.js code is supposed to be Fennec-agnostic. It's supposed to just take events and turn them into TapXXX events without concerning itself with content process / parent process.

Comment 6

[Benjamin Stover (:stechz)]

What is the actual code that is firing mousedown/mouseup/mousemove again?

Comment 7

[Wesley Johnston (:wesj)]

Taps occur at:

http://mxr.mozilla.org/mobile-browser/source/chrome/tests/browser_tapping.js#166

I'm not sure that this will fix this anyway. We should be seeing the same effect when the user taps. These are just going to be extremely close together.

Comment 8

[Benjamin Stover (:stechz)]

Attachment: permaorange: browser_tapping.js | Fired a good double tap

Comment 9

[Benjamin Stover (:stechz)]

I typed up a big comment for this, but bzexport failed me. It went something like...

The issue here is that our content process code regenerates mouse events, because the platform couldn't do this for us at first. Therefore, we needed code to handle the local browser case so that we don't have a mouse event feedback loop (see _targetIsContent in browser.js).

In this test, we can simply opt out of content stuff by sending the event to something that doesn't have the class "inputHandler."

In the future, Wes and I talked about using the platform's event forwarding to do the work for us, but we'll need a platform patch that takes async scrolling into consideration (this shouldn't be difficult to do). I'll file a bug.

Comment 10

[Benjamin Stover (:stechz)]

Filed bug 652240 for platform patch.

Comment 11

[Wesley Johnston (:wesj)]

Comment on attachment 527872 [details] [diff] [review]
permaorange: browser_tapping.js | Fired a good double tap

This seems fine to me, and its worth seeing if it fixes the test failures. Thanks!

Comment 12

[Benjamin Stover (:stechz)]

Pushed http://hg.mozilla.org/mozilla-central/rev/657cd15fa85d

Comment 13

[Phil Ringnalda (:philor)]

Still failing every time on Android, and most of the time but not quite every time on Maemo.

Comment 14

[Phil Ringnalda (:philor)]

Attachment: todo() on device

According to me last month, this'll make it an intermittent UNEXPECTED PASS on Maemo, but I checked the last 8 runs and none of them passed, so I think it'll be intermittent enough to live with.

Comment 15

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Pushed the test fix: http://hg.mozilla.org/mozilla-central/rev/ce933e1ea428

Comment 16

[Anna (Waverley)]

Can you provide some steps to reproduce in order to validate this bug?

Comment 17

[Andreea Pod]

I found this in tinderboxpushlog: TEST-KNOWN-FAIL | chrome://mochitests/content/browser/mobile/chrome/tests/browser_tapping.js | Fired a good double tap

I'm not sure if I can mark this bug as verified fixed.