Closed Bug 827325 Opened 7 years ago Closed 7 years ago

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

Categories

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

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 21
Tracking Status
firefox20 --- fixed
firefox21 --- fixed
fennec 20+ ---

People

(Reporter: aaronmt, Assigned: jchen)

References

Details

(Keywords: regression, reproducible)

Attachments

(1 file, 1 obsolete file)

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)
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.
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)
Looks like this is a Note II (4.1.2) based issue; still the same problem on 01/08 (mozilla-21).
(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!
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.
Summary: Regression: Unable to enter any URI; characters deleted following a colon → Regression: Unable to enter 'o' and 'p' in a URI; characters deleted following a colon
Jim, do you have a Note II? if not please request one.
tracking-fennec: ? → 20+
(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?
Flags: needinfo?(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).
Flags: needinfo?(rclement42)
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.
Attachment #707776 - Flags: review?(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. :)
Attachment #707776 - Flags: review?(cpeterson) → review+
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.
Status: NEW → ASSIGNED
(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 :)
Flags: in-testsuite-
Whiteboard: [leave open]
Target Milestone: --- → Firefox 21
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
Attachment #708136 - Flags: approval-mozilla-aurora?
Attachment #708136 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [leave open]
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.