Last Comment Bug 723480 - Single touch broken in Feb 1 2012 XUL Nightly
: Single touch broken in Feb 1 2012 XUL Nightly
Status: VERIFIED FIXED
: regression
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Firefox 12
: All All
: -- critical (vote)
: Firefox 13
Assigned To: Matt Brubeck (:mbrubeck)
:
:
Mentors:
: 723450 (view as bug list)
Depends on:
Blocks: 721484 723772
  Show dependency treegraph
 
Reported: 2012-02-02 05:17 PST by Robert Kaiser
Modified: 2012-02-06 20:01 PST (History)
9 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (1.08 KB, patch)
2012-02-02 16:53 PST, Matt Brubeck (:mbrubeck)
blassey.bugs: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Robert Kaiser 2012-02-02 05:17:13 PST
I just downloaded the XUL Nightly of Feb 1 (first with 13.0a1, but no real code changes in there yet from what I see) to my tablet and discovered that all single-touch actions (clicking links, trying to access tabs, trying to focus the awesomebar or open the menu) are broken. Two-finger pinch zoom works, though.

When I got the last 12.0a1 Nightly from Jan 31, this works fine, so the regression is within this one day of m-c. Given that practically no change landed on m-c between the Aurora uplift and the point where the Feb 1 Nightly was cut, I'd expect this problem to exist on Aurora 12 as well, but I haven't tested yet.
Comment 1 Robert Kaiser 2012-02-02 07:28:40 PST
OK, this is indeed affecting Aurora 12 XUL as well. As this might very well end up shipping on tablets, nominating for tracking.

Also, this means that bug 723200 has no influence on this, as it happens before this landed. And bug 603008 already landed way before the 12.0a1 build from Jan 31 that still works.
Comment 2 Robert Kaiser 2012-02-02 07:36:48 PST
Given the two Nightly builds I tested and looking at their changesets, the regression range would be:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3f26b7bee352&tochange=e18c7bc2c28e

As I tested that this is on today's Aurora 12 as well, we can ignore whatever came after the uplift, so that reduces the regression range to this:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3f26b7bee352&tochange=AURORA_BASE_20120131
Comment 3 Aaron Train [:aaronmt] 2012-02-02 09:59:57 PST
*** Bug 723450 has been marked as a duplicate of this bug. ***
Comment 4 Matt Brubeck (:mbrubeck) 2012-02-02 15:56:13 PST
The first bad revision is:
https://hg.mozilla.org/mozilla-central/rev/2fa163bb05d7
Wes Johnston <wjohnston@mozilla.com>
Bug 721484 - Don't disable mouse events for touch events. r=blassey
Comment 5 Matt Brubeck (:mbrubeck) 2012-02-02 16:53:03 PST
Created attachment 594018 [details] [diff] [review]
patch

This fixes the bug for me.
Comment 6 Wesley Johnston (:wesj) 2012-02-02 16:55:44 PST
Comment on attachment 594018 [details] [diff] [review]
patch

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

I'm not very comfortable reviewing this (I've messed this up twice now because I don't understand how these defines trickle around). Blassey?
Comment 7 Bill Gianopoulos [:WG9s] 2012-02-03 08:26:34 PST
(In reply to Matt Brubeck (:mbrubeck) from comment #5)
> Created attachment 594018 [details] [diff] [review]

Well not exactly perfect.  I can verify that this patch fixes the issue reported here.  However, we end up back with the issue reported in bug 721484.
Comment 8 Bill Gianopoulos [:WG9s] 2012-02-03 09:53:23 PST
(In reply to Bill Gianopoulos [:WG9s] from comment #7)
> (In reply to Matt Brubeck (:mbrubeck) from comment #5)
> > Created attachment 594018 [details] [diff] [review]
> 
> Well not exactly perfect.  I can verify that this patch fixes the issue
> reported here.  However, we end up back with the issue reported in bug
> 721484.

That said, from looking at config.status it appears this patch should have worked correctly in both cases.  Perhaps something else broke the testcase in bug 721484.  I am redoing my native feeenc build with this backed out to see what happens.
Comment 9 Bill Gianopoulos [:WG9s] 2012-02-03 10:16:18 PST
(In reply to Bill Gianopoulos [:WG9s] from comment #8)
> (In reply to Bill Gianopoulos [:WG9s] from comment #7)
> > (In reply to Matt Brubeck (:mbrubeck) from comment #5)
> > > Created attachment 594018 [details] [diff] [review]
> > 
> > Well not exactly perfect.  I can verify that this patch fixes the issue
> > reported here.  However, we end up back with the issue reported in bug
> > 721484.
> 
> That said, from looking at config.status it appears this patch should have
> worked correctly in both cases.  Perhaps something else broke the testcase
> in bug 721484.  I am redoing my native feeenc build with this backed out to
> see what happens.

Never mind.  I can't get that testcase to work in today's nightly Android native build either, which does not include this patch.  Must be a different issue.  Go figure ;-)
Comment 10 Matt Brubeck (:mbrubeck) 2012-02-03 10:48:42 PST
https://hg.mozilla.org/mozilla-central/rev/cadfebcc626e
Comment 11 Matt Brubeck (:mbrubeck) 2012-02-03 10:53:41 PST
Comment on attachment 594018 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): bug 721484

User impact if declined: XUL Fennec completely broken (does not respond to touch input).

Testing completed (on m-c, etc.): Tested locally by myself, wesj, and WG9s; landed on m-c February 3.

Risk to taking this patch (and alternatives if risky): Low risk.  This is a trivial fix to a build system bug that caused some code to be accidentally enabled in XUL Fennec when it never should have been.  This is effectively a partial backout of an accidental change.

String changes made by this patch: None.
Comment 12 Robert Kaiser 2012-02-04 06:27:38 PST
As the reporter, I think marking this as verified is OK - I can confirm that today's 13.0a1 XUL Nightly works on my TF101. Thanks for the fix!
Comment 13 Alex Keybl [:akeybl] 2012-02-05 14:00:45 PST
Comment on attachment 594018 [details] [diff] [review]
patch

[Triage Comment]
Important fix - approved for Aurora 12 and Beta 11.
Comment 14 Brad Lassey [:blassey] (use needinfo?) 2012-02-06 13:58:24 PST
https://hg.mozilla.org/releases/mozilla-beta/rev/8082febbe120
Comment 15 Mark Finkle (:mfinkle) (use needinfo?) 2012-02-06 20:01:34 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/cd3f31d128b8

Note You need to log in before you can comment on or make changes to this bug.