961612 - [AccessFu] Explore by Touch uses wrong coordinates on HiDPI Android devices

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Max Li [:maxli]]

Attachment: patch

Turns out one of the regressions that I was reacting to in Bug 890940 was caused by an actual bug (bug 924791), and not just an intentional change like the other ones. So when it was fixed, explore by touch coordinates are now incorrectly multiplied by the device pixel ratio, causing EBT on HiDPI devices to be pretty unusable.

Comment 1

[Marco Zehe (:MarcoZ)]

Thanks, maxLi! Marking this bug as a regression anyway since bug 924791 caused us to regress. And this also explains why I could never reproduce it, since I don't yet have high DPI devices here. We might be too late for the initial 27 release, or maybe not...But we should get this reviewed and landed ASAP so we can uplift.

Have you tested that this also always activates correct items still when double-tapping?

Comment 2

[Max Li [:maxli]]

(In reply to Marco Zehe (:MarcoZ) from comment #1)
> Have you tested that this also always activates correct items still when
> double-tapping?

Yes it does (which is unsurprising, since bug 924791 is about mouse/touch event coordinates, whereas the coordinates when double tapping are generated based upon the focused item)

Comment 3

[Marco Zehe (:MarcoZ)]

Comment on attachment 8362394 [details] [diff] [review]
patch

r=me. Stealing review so we can get this in at the earliest possible time, given the urgency of the release 27 being very close.

I tested this on a Nexus 7 2012 edition and a newly aquired Nexus 7 2013 edition, and it indeed fixes the problem of Explore By Touch, which I can reproduce on the 2013 Nexus 7 in Nightly builds.

Comment 4

[Marco Zehe (:MarcoZ)]

Landed on Inbound on MaxLi's behalf: https://hg.mozilla.org/integration/mozilla-inbound/rev/ce3e8f10d4d8

Comment 5

[Carsten Book [:Tomcat]]

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

Comment 6

[bhavana bajaj [:bajaj]]

Patch uplift on beta depends on how risky the patch is and the user reward here given we only have a final beta left. If this is absolutely low risk and there is less scope of fallout's and benefits users in a big way, please request uplift on beta with analysis. We would have to land it soon though so this can get in tomorrow on preparation for our Thursday's beta.

Else, we can wontfix for Fx27 but take an aurora uplift instead and resolve this in Fx28.

Comment 7

[Marco Zehe (:MarcoZ)]

Verified fixed in the 2nd nightly build made on 2014-01-21. Note that Firefox26 is not affected by this bug, so changing its flag accordingly.

Comment 8

[Marco Zehe (:MarcoZ)]

Comment on attachment 8362394 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 924791
User impact if declined: Blind users will no longer be able to explore web content with their finger on devices such as Nexus 5, Nexus 7 2013 edition, Samsung Galaxy S IV, and other High DPI Android devices, effectively breaking accessibility except for the very time consuming purely sequential swiping gesture.
Testing completed (on m-c, etc.): Yes.
Risk to taking this patch (and alternatives if risky): None. This just reverts a small part of bug 892940, which originally contained this workaround before bug 924791 was fixed. See comment #0 also.
String or IDL/UUID changes made by this patch: None.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5ff9dcd0fadd
https://hg.mozilla.org/releases/mozilla-beta/rev/7f439322cb45