827325 - Regression: Unable to enter 'o' and 'p' in a URI; characters deleted following a colon

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Keyboards and IME, defect)

Description

[Aaron Train [:aaronmt]]

Currently when one enters any URI, any character inserted following a colon is deleted. 

E.g, attempt to enter `about:privatebrowsing` → "about:p", yields "about"

This is likely to wind up on mozilla-20 (Aurora) in the next day.

--
Nightly (01/07)
Samsung Galaxy Note II (Android 4.1.2)

Comment 1

[Aaron Train [:aaronmt]]

Adding other findings from in-channel. With Smart Keyboard Pro, just another keyboard I have installed I get different behaviour; when I enter a colon after 'about', the character is never displayed. Attempting again, it *is* inserted, and anything after appears too.

Comment 2

[catalin.suciu]

Unable to reproduce on Samsung Galaxy Note (Android 4.0.4) and Samsung Galaxy Nexus (Android 4.1.2) using Nightly 20.0a1 (2013-01-07)

Comment 3

[Aaron Train [:aaronmt]]

Looks like this is a Note II (4.1.2) based issue; still the same problem on 01/08 (mozilla-21).

Comment 4

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

(In reply to Aaron Train [:aaronmt] from comment #3)
> Looks like this is a Note II (4.1.2) based issue; still the same problem on
> 01/08 (mozilla-21).

Can you try to get a regression range? There don't seem to be many Note IIs around. Thanks!

Comment 5

[Aaron Train [:aaronmt]]

I've downloaded builds back to early November and this issue is still there. I've narrowed this down to being an issue with really two characters only: 'o' and 'p'. I stumbled upon this with the new 'about:privatebrowsing' URI.

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

Jim, do you have a Note II? if not please request one.

Comment 7

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

(In reply to Brad Lassey [:blassey] from comment #6)
> Jim, do you have a Note II? if not please request one.

Okay. I only have the original Note.


rclement, do you have this problem on the Note II?

Comment 8

[rclement42]

(In reply to Jim Chen [:jchen :nchen] from comment #7)
> (In reply to Brad Lassey [:blassey] from comment #6)
> > Jim, do you have a Note II? if not please request one.
> Okay. I only have the original Note.
> rclement, do you have this problem on the Note II?

Using Mozilla 18 (Jan 8) and Mozilla beta (Jan 10)

1. Using Samsung stock keyboard;

Trying to enter "about:plugins"; after I type in 'p', the colon and p get both deleted, resulting in aboutlugins (fine with 'c' like "about:config" or 'h'.  Couldn't think of an 'o' option to try?)

2. Using Swype as keyboard (my usual)

The colon doesn't get entered the first time if I try to input it from a long keypress (it does the second time, and then I have no problems entering anything afterwards, as reported in Comment 1).  Interesting to note that if I input the colon through the symbol keyboard (toggle in Swype) instead, then it goes in first try and no issues afterwards.  (For the Samsung stock keyboard, one has to toggle to the symbol keyboard; no long keypress to symbol).

Comment 9

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

Attachment: Delay calling restartInput in AwesomeBar to avoid specific IME quirks (v1)

This bug looks like a bug in the Samsung keyboard.

This patch adds a delay to calling restartInput for specific IMEs like Samsung keyboard and Swiftkey, to workaround their buggy behavior.

Comment 10

[Chris Peterson [:cpeterson]]

Comment on attachment 707776 [details] [diff] [review]
Delay calling restartInput in AwesomeBar to avoid specific IME quirks (v1)

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

LGTM with some suggestions

::: mobile/android/base/AwesomeBar.java
@@ +327,5 @@
>          mGoButton.setImageResource(imageResource);
>          mGoButton.setContentDescription(contentDescription);
>  
> +        InputMethodManager imm = (InputMethodManager)
> +            mText.getContext().getSystemService(Context.INPUT_METHOD_SERVICE);

Can you just call InputMethods.getInputMethodManager(mText.getContext())?

@@ +330,5 @@
> +        InputMethodManager imm = (InputMethodManager)
> +            mText.getContext().getSystemService(Context.INPUT_METHOD_SERVICE);
> +        if (imm == null) {
> +            return;
> +        }

AFAICT, getSystemService(Context.INPUT_METHOD_SERVICE) will never return a null InputMethodManager. We should avoid adding new `imm == null` checks.

@@ +337,5 @@
>              int optionBits = mText.getImeOptions() & ~EditorInfo.IME_MASK_ACTION;
>              mText.setImeOptions(optionBits | imeAction);
> +
> +            mDelayRestartInput = InputMethods.shouldDelayAwesomebarUpdate(
> +                InputMethods.getCurrentInputMethod(this));

Let's just move the InputMethods.getCurrentInputMethod(this) call into the InputMethods.shouldDelayAwesomebarUpdate() method itself.

::: mobile/android/base/InputMethods.java
@@ +94,5 @@
>      }
> +
> +    public static boolean shouldDelayAwesomebarUpdate(String inputMethod) {
> +        return METHOD_SWIFTKEY.equals(inputMethod) ||
> +               METHOD_SAMSUNG.equals(inputMethod);

Let's list METHOD_SAMSUNG before METHOD_SWIFTKEY. The Samsung keyboard will be much more common than SwiftKey and Samsung comes first alphabetically. :)

Comment 11

[Chris Peterson [:cpeterson]]

jchen, please ignore my comment about removing the `imm == null` check. According to bug 777505, getInputMethodManager() can return null if the View or Context is null.

Comment 12

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

(In reply to Chris Peterson (:cpeterson) from comment #11)
> jchen, please ignore my comment about removing the `imm == null` check.
> According to bug 777505, getInputMethodManager() can return null if the View
> or Context is null.

Alright. I started null-checking everything after a string of NPE regressions from some of my recent patches :)

Comment 13

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

Attachment: Delay calling restartInput in AwesomeBar to avoid specific IME quirks (v2)

Addressed review comments.

https://hg.mozilla.org/integration/mozilla-inbound/rev/920d24c1a157

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/920d24c1a157

Comment 15

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

Comment on attachment 708136 [details] [diff] [review]
Delay calling restartInput in AwesomeBar to avoid specific IME quirks (v2)

[Approval Request Comment]

Bug caused by (feature/regressing bug #): N/A

User impact if declined: Bad user experience: missing letters when typing in the awesomebar.

Testing completed (on m-c, etc.): Locally, m-c

Risk to taking this patch (and alternatives if risky): Small; only awesomebar behavior is changed

String or UUID changes made by this patch: None

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b19b94364f01