704579 - No focus selection highlight on tap and hold on links

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

Currently when one taps and holds on an element such as a link, there is no selection highlight 'ring' around the element: example, tap and hold on any of the directories here http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/birch-android/

There should be a focus selection highlight on tap and hold.

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch

Patch moves all of our clicking code to use java native handlers. That introduces some delay between when you tap and when clicks fire, but I'll need to introduce it anyway to listen for double taps.

I had to do a lot of weird coordinate conversions here, and I'm not that sure about them. When we fire clicks, we need to make sure we're using window coordinates, which is what we're sending from Android to content. When we call things like DOMWindowUtils.elementFromPoint and getBoundingClientRect, we need/get results in... different coordinate systems.

Comment 2

[Chris Lord [:cwiiis]]

Comment on attachment 577895 [details] [diff] [review]
Patch

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

r+ if the two following transformations can't be avoided and are correct.

::: mobile/android/chrome/content/browser.js
@@ +1473,5 @@
> +    }
> +    let viewport = tab.viewport;
> +    return [
> +        ((aX-tab.viewportExcess.x)*viewport.zoom + viewport.offsetX),
> +        ((aY-tab.viewportExcess.y)*viewport.zoom + viewport.offsetY)

This looks correct, but I'm not really sure... Couldn't the event be delivered to the window instead of the browser element? That way, Gecko would do the transformations for us.

If events appear correct while zoomed in on a long page both at the top-left, middle and bottom-right, then this must be correct - it's easier to test it than to work through it.

@@ +1486,5 @@
> +    }
> +    let viewport = tab.viewport;
> +    return [
> +        (aX - viewport.offsetX)/viewport.zoom + tab.viewportExcess.x,
> +        (aY - viewport.offsetY)/viewport.zoom + tab.viewportExcess.y

Same here.

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch for checkin

Comment 4

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

https://hg.mozilla.org/projects/birch/rev/548fd9badc87

Comment 5

[Aaron Train [:aaronmt]]

Samsung Nexus S (Android 2.3.6)
20111201040252
http://hg.mozilla.org/projects/birch/rev/d71c91775f9b