Accented characters are substituted when entering other characters and can also remove the content before it on mobile.twitter.com when using HKB

VERIFIED FIXED in Firefox 14

Status

()

defect
VERIFIED FIXED
7 years ago
3 years ago

People

(Reporter: AdrianT, Assigned: cpeterson)

Tracking

({regression})

15 Branch
Firefox 15
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox14 verified, firefox15 verified, firefox16 verified, firefox17 verified, blocking-fennec1.0 +)

Details

(Whiteboard: HKB, )

Attachments

(2 attachments)

Reporter

Description

7 years ago
Nightly 15.0a1 2012-05-18 / Aurora 14.0a2 2012-05-18
Device: HTC Desire Z / Motorola Droid Pro
OS: Android 2.3.3 / Android 2.3.4

Steps to reproduce:
1. Go to mobile.twitter.com.
2. Sign in. In the "What's happening?" field start typing some text.
3. Pres and hold a key to bring up the alternative values (use a,e,i,c or o which have accented characters) and choose one.
4. Continue typing.

Expected results:
The user can type words with accented characters in them. 

Actual results:
Sometimes when the accented character is entered everything before it is removed. When entering characters after the accented character the accented character is substituted.

Please see the video capture: http://youtu.be/ZCyDxwJ4v0U
Reporter

Updated

7 years ago
Blocks: 743468
Reporter

Updated

7 years ago
Summary: Accented characters are substituted when entering other characters and can also remove the content before it on mobile.twitter.com → Accented characters are substituted when entering other characters and can also remove the content before it on mobile.twitter.com when using HKB
Assignee

Updated

7 years ago
Assignee: nobody → cpeterson
Assignee

Comment 1

7 years ago
Is this a regression? I thought I had fixed this bug on Nightly 2012-04-28, but I can now repro on Droid Pro.
Whiteboard: HKB
Reporter

Comment 2

7 years ago
The issue was fixed in Nightly 2012-05-16 and has reappeared on Nightly 2012-05-18.

Possible regression range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=762e95608da3&tochange=e794cef56df6
Assignee

Comment 3

7 years ago
Thanks, Adrian. In that regression range, I suspect this Nightly regression was my fix for bug 751513:

https://hg.mozilla.org/mozilla-central/rev/43aa1f392da4
blocking-fennec1.0: --- → ?
Let's test aurora and beta to make sure this is not showing up there. It should be backed out from those channels.
Keywords: qawanted
(In reply to Mark Finkle (:mfinkle) from comment #4)
> Let's test aurora and beta to make sure this is not showing up there. It
> should be backed out from those channels.

Adrian, can you retest as above?  if its fixed on those channels, comment and remove qawanted.
Reporter

Comment 6

7 years ago
The issue is reproducible on Firefox Beta 14.0b2 and Aurora 14.0a2 2012-05-21.
Keywords: qawanted

Updated

7 years ago
No longer blocks: 743468
blocking-fennec1.0: ? → +
Assignee

Comment 7

7 years ago
Commit composition string when IME is reinitialized from VKB popup window.

1. onCreateInputConnection() can be called during composition when input focus is restored from a VKB popup window (such as for entering accented characters) back to our IME. We want to commit our active composition string.

2. If Gecko cancels the current composition from underneath us, abandon our active composition string WITHOUT committing it.
Attachment #628973 - Flags: review?(blassey.bugs)
Assignee

Comment 8

7 years ago
Assert sanity of composition string's state changes. These asserts will fail without the fix from patch part 1.

I used MOZ_ASSERT (which is fatal in debug builds) for conditions I know should be true. I used NS_ASSERTION (which merely logs warnings in debug builds) for conditions I think are true, but I'm not completely sure yet.
Attachment #628977 - Flags: review?(blassey.bugs)
Assignee

Updated

7 years ago
Status: NEW → ASSIGNED
Comment on attachment 628973 [details] [diff] [review]
part-1-commit-on-reinitialization.patch

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

::: mobile/android/base/GeckoInputConnection.java
@@ +534,5 @@
> +    private void resetCompositionState() {
> +        if (DEBUG && hasCompositionString()) {
> +            Log.d(LOGTAG, "resetCompositionState() is abandoning an active composition string, "
> +                          + "mCompositionStart=" + mCompositionStart + " -> -1");
> +        }

why can't this go in the debug subclass?

@@ -1266,5 @@
> -    @Override
> -    public void notifyIMEChange(String text, int start, int end, int newEnd) {
> -        Log.d(LOGTAG, String.format("IME: >notifyIMEChange(\"%s\", start=%d, end=%d, newEnd=%d)",
> -                                    text, start, end, newEnd));
> -        super.notifyIMEChange(text, start, end, newEnd);

why remove this logging?
Attachment #628973 - Flags: review?(blassey.bugs) → review+
Attachment #628977 - Flags: review?(blassey.bugs) → review+
Assignee

Comment 10

7 years ago
(In reply to Brad Lassey [:blassey] from comment #9)

> > +    private void resetCompositionState() {
> > +        if (DEBUG && hasCompositionString()) {
> > +            Log.d(LOGTAG, "resetCompositionState() is abandoning an active composition string, "
> > +                          + "mCompositionStart=" + mCompositionStart + " -> -1");
> > +        }
> 
> why can't this go in the debug subclass?

Good point. I've fix that.


> > -    public void notifyIMEChange(String text, int start, int end, int newEnd) {
> > -        Log.d(LOGTAG, String.format("IME: >notifyIMEChange(\"%s\", start=%d, end=%d, newEnd=%d)",
> > -                                    text, start, end, newEnd));
> > -        super.notifyIMEChange(text, start, end, newEnd);
> 
> why remove this logging?

For what seems like convenience of not adding a new function, Gecko calls notifyIMEChange() for both text changes and selection changes. A negative `newEnd` sentinel value indicates a selection change. The GeckoInputConnection super class demultiplexes notifyIMEChange() to notifyIMETextChange() or notifyIMESelectionChange(). Since those functions log their own more-specific calls, I removed the redundant less-specific logging in notifyIMEChange().
Assignee

Comment 12

7 years ago
Comment on attachment 628973 [details] [diff] [review]
part-1-commit-on-reinitialization.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: HKB users will not be able to enter accented characters.
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): Low risk of incomplete composition strings being entered. Android Java only. blocking-fennec1.0=+
String or UUID changes made by this patch: N/A

Patch part 2 is NOT needed for Aurora because it just adds some debug code.
Attachment #628973 - Flags: approval-mozilla-aurora?

Comment 13

7 years ago
https://hg.mozilla.org/mozilla-central/rev/9a6a5cc5a4a5
https://hg.mozilla.org/mozilla-central/rev/180b78b6cec4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
Comment on attachment 628973 [details] [diff] [review]
part-1-commit-on-reinitialization.patch

[Triage Comment]
Attachment #628973 - Flags: approval-mozilla-aurora? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.