828990 - Thumb position does not advance; and or, unnecessary thumb position in input fields on tap

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

Currently when one taps into an input field to focus it, a thumb position appears. Inserting characters into the field does not automatically advance the thumb position. 

I expected either the thumb position should advance, or otherwise not have it display at all.

--
Nightly (01/10)
Samsung Galaxy Nexus (Android 4.1.2)

Comment 1

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

I think we should hide the thumb if the user starts typing.

Comment 2

[Aaron Train [:aaronmt]]

This is now on Beta; silly imo

Comment 3

[Brian Nicholson (:bnicholson)]

Attachment: Hide thumb on compositionend

Hiding the thumb on compositionend seems to work fine from general testing - any edge cases that I should be aware of?

I also removed _elementReceivesInput and _isEditable since they appear to be dead code.

Comment 4

[Brian Nicholson (:bnicholson)]

CC'ing Margaret in case she has any feedback.

Comment 5

[Brian Nicholson (:bnicholson)]

Comment on attachment 726957 [details] [diff] [review]
Hide thumb on compositionend

Err, sorry...didn't quite finish this yet.

Comment 6

[Brian Nicholson (:bnicholson)]

Attachment: Hide thumb on compositionend, v2

cleaned up

Comment 7

[Brian Nicholson (:bnicholson)]

Comment on attachment 726991 [details] [diff] [review]
Hide thumb on compositionend, v2

Oops, wrong reviewer!

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 726991 [details] [diff] [review]
Hide thumb on compositionend, v2

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

Can someone else review this too? The patch looks okay to me, although I'm not familiar with browser.js

::: mobile/android/chrome/content/browser.js
@@ -4649,5 @@
>    },
>  
> -  _elementReceivesInput: function(aElement) {
> -    return aElement instanceof Element &&
> -        kElementsReceivingInput.hasOwnProperty(aElement.tagName.toLowerCase()) ||

I think you can remove kElementsReceivingInput too?

Comment 9

[Brian Nicholson (:bnicholson)]

Comment on attachment 726991 [details] [diff] [review]
Hide thumb on compositionend, v2

Yeah, I probably should have included margaret as a reviewer anyway -- just wanted to make sure the compositionend IME event will work as intended.

Comment 10

[:Margaret Leibovic]

Comment on attachment 726991 [details] [diff] [review]
Hide thumb on compositionend, v2

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

LGTM. Jim's right that you can get rid of kElementsReceivingInput.

Comment 11

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/af59a6b444f0

Comment 12

[Ed Morley [:emorley]]

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

Comment 13

[Brian Nicholson (:bnicholson)]

Comment on attachment 726991 [details] [diff] [review]
Hide thumb on compositionend, v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none
User impact if declined: thumb stays in fixed position when typing, which looks broken
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none

Comment 14

[bhavana bajaj [:bajaj]]

Comment on attachment 726991 [details] [diff] [review]
Hide thumb on compositionend, v2

low risk, mostly clean-up - hides the thumb when you start inserting character in a input field.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/90b3c33b1edd