775759 - Native handles don't precisely line up horizontally with the ends of the selection

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Text Selection, defect)

Description

[:Margaret Leibovic]

There's probably some style issue we need to fix in Java. It looks like the image itself may have some transparent space on the edge.

Comment 1

[:Margaret Leibovic]

One of the issues here is that the images have shadows on their taller sides, and the edge of the shadow is lining up with the edge of the selection. I tried adding an offset to the margin to account for this, but it can still be off by varying amounts at times, and I'm wondering if there's some issue with our coordinate conversions. I'm testing right now with the patch for bug 775722 applied.

Comment 2

[:Margaret Leibovic]

Attachment: account for shadow in horizontal margins

This doesn't fix the issue I was seeing with the more random mispositioning of the handles, but it does help generally get them closer to the right spot.

I picked 2dp kind of randomly because it seemed to work. Ian, is there a more correct way to be choosing this value? (Density-independent pixels always confuse me.)

Comment 3

[Ian Barlow (:ibarlow)]

Well, whatever number is the one that fixes it really ;)

--

But in terms of understanding DIPs, a (somewhat) simple way of looking at it is:

On MDPI screens, 2 DIP = 2 actual pixels
For HDPI screens, 2 DIP = 3 actual pixels
For XHDPI screens, 2 DIP = 4 actual pixels

Or in other words, the relative density of MDPI / HDPI / XHDPI is 1x / 1.5x / 2x

This way if you know what kind of phone you have, you can look at a screenshot and work out, based on actual pixels, how many DIPs you need to adjust something to make it work. 

Hopefully that's helpful and not more confusing. FWIW I can barely keep up even on good days ;)

Comment 4

[:Margaret Leibovic]

Cool, well I think 2dp is right then, since the MDPI image we use has 2px of shadow.

Comment 5

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/61666a1574f2

Comment 6

[:Margaret Leibovic]

Comment on attachment 644523 [details] [diff] [review]
account for shadow in horizontal margins

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 774938 (native text selection handles)
User impact if declined: handles don't exactly line up with the selection ends
Testing completed (on m-c, etc.): just landed on inbound
Risk to taking this patch (and alternatives if risky): low-risk style change
String or UUID changes made by this patch: n/a

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 644523 [details] [diff] [review]
account for shadow in horizontal margins

approved for beta/aurora as part of the dep bugs for native handles for text selection.

Comment 8

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/33799e1ebccf
https://hg.mozilla.org/releases/mozilla-beta/rev/2e976c303c07

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/61666a1574f2