Closed
Bug 822558
Opened 11 years ago
Closed 11 years ago
TouchEvent.changedTouches[] includes unchanged touches
Categories
(Core :: DOM: Events, defect)
Tracking
()
People
(Reporter: djf, Assigned: cjones)
Details
(Keywords: regression)
Attachments
(1 file)
1.40 KB,
patch
|
mwu
:
review+
|
Details | Diff | Splinter Review |
Bug #785554 is back in FirefoxOS! Steps to reproduce: 0) run adb logcat | grep Console 1) Open gallery app 2) Tap on a thumbnail to view an image 3) Put one finger down on the image 4) Put another finger down on the image 5) Lift either one of your fingers You'll see this message in logcat: gesture_detector.js: spurious extra changed touch on touchend. See https://bugzilla.mozilla.org/show_bug.cgi?id=785554 This message is from the gesture_detector.js module and it is printed when the module gets a touchend event with two touches in its changedTouches array. But it happened when you lifted just one finger off the screen! The message dates back to the original bug #785554. This bug is now also affecting the Gaia keyboard, which has been converted to use multitouch. See https://bugzilla.mozilla.org/show_bug.cgi?id=806540#c31 To be honest, I'd forgotten that this bug had been fixed. I remember noticing the "spurious extra touch" message in logcat a couple of times last week (or maybe earlier than that). Chris: could this regression be related to https://bugzilla.mozilla.org/show_bug.cgi?id=774458 or to any of the recent event work related to that bug?
Reporter | ||
Comment 1•11 years ago
|
||
cc'ing the people who were cc'ed on the original bug, and Margaret, whose keyboard patch is affected by it now.
Reporter | ||
Updated•11 years ago
|
Keywords: regression
Assignee | ||
Comment 2•11 years ago
|
||
(In reply to David Flanagan [:djf] from comment #0) > Chris: could this regression be related to > https://bugzilla.mozilla.org/show_bug.cgi?id=774458 or to any of the recent > event work related to that bug? Nope, that code doesn't participate in scrolling in this test case. I'm now wondering if this regression is independent from bug 814252, but they've conspired together to break things.
Assignee | ||
Comment 3•11 years ago
|
||
Neither bug 814252 nor bug 815943 caused this.
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to David Flanagan [:djf] from comment #0) > Bug #785554 is back in FirefoxOS! > > Steps to reproduce: > > 0) run adb logcat | grep Console > 1) Open gallery app > 2) Tap on a thumbnail to view an image > 3) Put one finger down on the image > 4) Put another finger down on the image > 5) Lift either one of your fingers > In addition, we're firing two touchend events for the single finger lift, and both have multiple touches in changedTouches.
Assignee | ||
Comment 5•11 years ago
|
||
I see this in a 12-6 nightly build, so it's not new.
Assignee | ||
Comment 6•11 years ago
|
||
good: git://github.com/mozilla/releases-mozilla-aurora 607313557a95af6b3d56cf8b211237906ed497b3 bad: http://git.mozilla.org/releases/gecko 372f36fb1a5d6428bcba336499abe5c2d0105433 /me gets really scared ...
Assignee | ||
Comment 7•11 years ago
|
||
https://github.com/mozilla/releases-mozilla-aurora/compare/607313557a95af6b3d56cf8b211237906ed497b3...688429b8eb7bc26f2ae13905b33e7f4839997ef4 makes me even scaredier.
Assignee | ||
Comment 8•11 years ago
|
||
This still works on m-c, but somehow it has regressed on the branches. Sigh.
Summary: TouchEvent.changedTouches[] includes unchanged touches again → TouchEvent.changedTouches[] includes unchanged touches on aurora and b2g18
Assignee | ||
Comment 9•11 years ago
|
||
Bug 814252 works just fine on mozilla-central. I suspect that whatever regressed this bug also caused bug 814252 to break on branches. That's we need to develop against gecko-18 / b2g18 :(. So now this blocks a blocker.
Assignee | ||
Comment 10•11 years ago
|
||
Facts here are - fix originally landed to m-c - fix rode the train to aurora when it was uplifted - as of 11/19, regression not in aurora - as of the first b2g build made from beta, 11/20, regression in beta - regression currently in aurora - current m-c is ok So we have these regression ranges - aurora between 11/19 and now - beta between last uplift and 11/20. I think this is the narrower range but need to double check the uplift date. It's really bizarre that this isn't a problem on current m-c :/.
Assignee | ||
Comment 11•11 years ago
|
||
Looks like the uplift was 11/19 ...
Assignee | ||
Comment 12•11 years ago
|
||
A regression range I'm pretty sure is correct is https://github.com/mozilla/mozilla-central/compare/aa21673a358313147a3042c926314dea56e81b23...37b16e9fedae9f402746be4e10a088d448214d42 A smaller one that that I'm less sure about is https://github.com/mozilla/mozilla-central/compare/25118313f6e115fab45a525925ae90dc3975cd92...37b16e9fedae9f402746be4e10a088d448214d42 Nothing suspicious in either, except for bug 796452, but it was also backed out in the first range ...
Assignee | ||
Comment 13•11 years ago
|
||
(In reply to Chris Jones [:cjones] [:warhammer] from comment #6) > good: > git://github.com/mozilla/releases-mozilla-aurora > 607313557a95af6b3d56cf8b211237906ed497b3 > More bad things here ... a build made from hg.m.o/aurora from what should be the same commit is bad. Taking a long time to iron out.
Assignee | ||
Comment 14•11 years ago
|
||
A local build from <project name="gaia" path="gaia" remote="b2g" revision="41b694e189e806b6654d813722dd6d8b2e8d8d4e"/> <project name="releases-mozilla-aurora" path="gecko" remote="mozilla" revision="b3dcea6f682ce40f4ac32d8035f65971dbc2115e"/> has the regression, but flashing the 11-15 nightly that manifest was made from is clean. More badness.
Assignee | ||
Comment 15•11 years ago
|
||
Flashing just the gecko build made locally from the manifest is OK. Same with the gaia build.
Assignee | ||
Comment 16•11 years ago
|
||
(In reply to Chris Jones [:cjones] [:warhammer] from comment #6) > bad: > http://git.mozilla.org/releases/gecko > 372f36fb1a5d6428bcba336499abe5c2d0105433 Flashing just gecko built from this on top of 11-15 is good.
Updated•11 years ago
|
Target Milestone: --- → B2G C3 (12dec-1jan)
Comment 17•11 years ago
|
||
Assigning to Chris for now since he is working on it, let us know if you need this load balanced to someone else by unassigning it.
Assignee: nobody → jones.chris.g
Assignee | ||
Comment 18•11 years ago
|
||
Indeed, this doesn't reproduce with the *most recent* gecko-18 build flashed on top of the 11-15 nightly. (Yay that old gaia works with new gecko.) That means either there's a kernel issue or something in the non-gecko environment. Was anyone who filed regressions against bug 814252 testing with unagi?
Comment 19•11 years ago
|
||
(In reply to Chris Jones [:cjones] [:warhammer] from comment #18) > Was anyone who filed regressions against bug 814252 testing with unagi? The "'contextmenu' triggers even if scrolling was performed" regression filed as bug 821864 in the e-mail was experienced (by dietrich) and reproduced (by me) on auto-updated unagi. I just auto-updated to the 20121219070201 nightly on unagi and the 'contextmenu' regression appears to have disappeared.
Assignee | ||
Comment 20•11 years ago
|
||
Good to know, thanks. Not an otoro/unagi difference. The regressing patch was backed out, btw.
Assignee | ||
Comment 21•11 years ago
|
||
Aha 1. Flash 11/15 build. No bug. 2. |./flash.sh gecko| for 0e660766a5581caee81cc1def6a098d0d37a9083 (12/19). No bug. 3. |make reset-gaia| then reboot for d38689acc60bddc6e54deda688a82e2777e9826c (12/19). Bug. Now I reproduce.
Assignee | ||
Comment 22•11 years ago
|
||
1. Flash 11/15 build and enable console in settings app. Bug. So this regressed before the oldest nightly I have.
Assignee | ||
Comment 23•11 years ago
|
||
(In reply to Chris Jones [:cjones] [:warhammer] from comment #10) > - current m-c is ok (Re-checked for sanity and console seems to be enabled by default in latest builds. Disable+enable doesn't show the "assertion" failure in logcat. This also matches the observation that bug 814252 didn't regress the same way on m-c.)
Assignee | ||
Comment 24•11 years ago
|
||
The problem exists as of 10/11.
Comment 25•11 years ago
|
||
Bug 790454 landed early November. Could it have caused this?
Assignee | ||
Comment 26•11 years ago
|
||
Right now it's looking like bug 785554 didn't actually fix the original problem. Bug 790454 might actually have *fixed* it ;).
Assignee | ||
Comment 27•11 years ago
|
||
The parent process is lying to the content process First finger down I/Gecko ( 526): [526] Starting dispatch of touchstart ... I/Gecko ( 526): touch ID 0 (changed? yes) Second finger down I/Gecko ( 526): [526] Starting dispatch of touchstart ... I/Gecko ( 526): touch ID 1 (changed? yes) I/Gecko ( 526): touch ID 0 (changed? no) Second finger up I/Gecko ( 526): [526] Starting dispatch of touchend ... I/Gecko ( 526): touch ID 0 (changed? yes) I/Gecko ( 526): touch ID 1 (changed? yes) Touch 0 didn't change.
Assignee | ||
Comment 28•11 years ago
|
||
Alas, bug 790454 was uplifted already.
Assignee | ||
Comment 29•11 years ago
|
||
First finger down I/Gecko ( 605): TOUCH [nsAppShell][605] Starting dispatch of touchstart ... I/Gecko ( 605): touch ID 0 (changed? no) I/Gecko ( 667): TOUCH [TabChild][667] Starting dispatch of touchstart ... I/Gecko ( 667): touch ID 0 (changed? yes) makes sense so far ... Second finger down I/Gecko ( 605): TOUCH [nsAppShell][605] Starting dispatch of touchstart ... I/Gecko ( 605): touch ID 1 (changed? no) I/Gecko ( 605): touch ID 0 (changed? no) I/Gecko ( 667): TOUCH [TabChild][667] Starting dispatch of touchstart ... I/Gecko ( 667): touch ID 1 (changed? yes) I/Gecko ( 667): touch ID 0 (changed? no) still sensical. Second finger up I/Gecko ( 605): TOUCH [nsAppShell][605] Starting dispatch of touchend ... I/Gecko ( 605): touch ID 1 (changed? no) I/Gecko ( 667): TOUCH [TabChild][667] Starting dispatch of touchend ... I/Gecko ( 667): touch ID 0 (changed? yes) I/Gecko ( 667): touch ID 1 (changed? yes) Gonk widgetry is reporting the right thing (it doesn't have to track changed touches), but something in the DOM code is getting the changed list wrong.
Assignee | ||
Comment 30•11 years ago
|
||
And we have a winner, TabParent is confusing remote content I/Gecko ( 739): TOUCH [nsAppShell][739] Starting dispatch of touchend ... I/Gecko ( 739): TOUCH touch ID 1 (changed? no) I/Gecko ( 739): TOUCH [TabParent][739] Starting dispatch of touchend ... I/Gecko ( 739): TOUCH touch ID 0 (changed? no) I/Gecko ( 739): TOUCH touch ID 1 (changed? yes) I/Gecko ( 811): TOUCH [TabChild][811] Starting dispatch of touchend ... I/Gecko ( 811): TOUCH touch ID 0 (changed? yes) I/Gecko ( 811): TOUCH touch ID 1 (changed? yes)
Assignee | ||
Comment 31•11 years ago
|
||
I have a fix for this bug (which we should take), but this bug isn't causing bug 814252.
Assignee | ||
Comment 32•11 years ago
|
||
Third-party content is affected by this because we will dispatch crazy touch series to them. It will undoubtedly break like our gallery app did. The patch is small and safe.
Attachment #694086 -
Flags: review?(mwu)
Updated•11 years ago
|
blocking-basecamp: ? → +
Assignee | ||
Comment 33•11 years ago
|
||
(The reason the test didn't fail on m-c was because touch events were completely broken.)
Summary: TouchEvent.changedTouches[] includes unchanged touches on aurora and b2g18 → TouchEvent.changedTouches[] includes unchanged touches
Updated•11 years ago
|
Attachment #694086 -
Flags: review?(mwu) → review+
Assignee | ||
Comment 34•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cbe2966ec8a
Comment 35•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8cbe2966ec8a
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 36•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/8d4dacd6805e https://hg.mozilla.org/releases/mozilla-b2g18/rev/a93cb30fc761
You need to log in
before you can comment on or make changes to this bug.
Description
•