988007 - unable to place cursor in front of the first character on a line

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Dietrich Ayala (:dietrich)]

... resulting in maddening fury.

Master (1.5), Nexus 4, built 3/24. (Though I've seen this before.)

STR:

1. compose an email with a few lines of text
2. try to place the cursor before the first character on a line

It's impossible. Even the 2nd character was hard - I was able to do it less than 50% of the time.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

We currently have margin set on .cmp-body text on top/right/left, but padding is set to zero.  We might be able to help out the platform here by switching that from margin to padding.

If that does not work then we already have magic helper logic that turns clicks above/below the contenteditable area into seeks to the first and last characters in the body.  It could be amended to handle clicks outside the body area but within the vertical space as line seeks.  A fairly straightforward caretPositionFromPoint after adjusting the x-coordinate would work, and https://developer.mozilla.org/en-US/docs/Web/API/Document.caretPositionFromPoint even has example code.  Yay MDN!

cc'ing the current reigning hero of the body compose area to whom many celebratory beverages are already due.

Comment 2

[Kevin Grandon :kgrandon]

Attachment: Github pull request

Took a quick look because I was curious. I tried switching to padding instead of margin and it seemed to be much more responsive, I believe this gives us an extra ~15px of touch area on the left and right of each line, and ~10px above the first line. This should make focusing more precise.

I don't think there's need to implement caretPositionFromPoint yet - though I think there are other improvements we can do here. It seems like we can also currently focus on a "border" between fields and lose the keyboard - it would probably be better to then focus on the above input. I think we can accomplish this with some clever usage of :before or :after pseudo classes, though we should do this in another bug.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8396773 [details] [review]
Github pull request

(In reply to Kevin Grandon :kgrandon from comment #2)
> Created attachment 8396773 [details] [review]
> Github pull request
> 
> Took a quick look because I was curious. I tried switching to padding
> instead of margin and it seemed to be much more responsive, I believe this
> gives us an extra ~15px of touch area on the left and right of each line,
> and ~10px above the first line. This should make focusing more precise.

Trying on my buri, I didn't actually have a major problem before this patch, but it does seem a little bit better.  Even if the performance were somehow the same your patch definitely seems like the best state of things to give the platform a fighting chance.
 
> I don't think there's need to implement caretPositionFromPoint yet - though
> I think there are other improvements we can do here. It seems like we can
> also currently focus on a "border" between fields and lose the keyboard - it
> would probably be better to then focus on the above input. I think we can
> accomplish this with some clever usage of :before or :after pseudo classes,
> though we should do this in another bug.

Note that we already have some JS logic in place to try and improve focusing behaviour including cursor position:
https://github.com/mozilla-b2g/gaia/blob/master/apps/email/js/cards/compose.js#L34

It's possible that we could improve things in general by replacing the JS with more DOM magic, but be super-aware that because of how those bubbles currently work, the text field gets dynamically sized and moved around, so it's important to test with bubbles in place.  A cleanup of bubble handling may also be required for this reason.


Thanks! r=asuth

Comment 4

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/gaia/commit/2df4befff9b7ec02fa3f2298eccc72984c3173ef

Dietrich - let us know how it performs after this patch. Thanks!