723480 - Single touch broken in Feb 1 2012 XUL Nightly

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Robert Kaiser]

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]

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]

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 4

[Matt Brubeck (:mbrubeck)]

The first bad revision is:
https://hg.mozilla.org/mozilla-central/rev/2fa163bb05d7
Wes Johnston <[e-mail]>
Bug 721484 - Don't disable mouse events for touch events. r=blassey

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This fixes the bug for me.

Comment 6

[Wesley Johnston (:wesj)]

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]]

(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]]

(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]]

(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)]

https://hg.mozilla.org/mozilla-central/rev/cadfebcc626e

Comment 11

[Matt Brubeck (:mbrubeck)]

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]

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]]

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?)]

https://hg.mozilla.org/releases/mozilla-beta/rev/8082febbe120

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/cd3f31d128b8