Closed Bug 756429 Opened 13 years ago Closed 13 years ago

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

Categories

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

15 Branch
ARM
Android
defect
Not set
normal

Tracking

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

VERIFIED FIXED
Firefox 15
Tracking Status
firefox14 --- verified
firefox15 --- verified
firefox16 --- verified
firefox17 --- verified
blocking-fennec1.0 --- +

People

(Reporter: AdrianT, Assigned: cpeterson)

References

()

Details

(Keywords: regression, Whiteboard: HKB)

Attachments

(2 files)

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
Blocks: 743468
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: nobody → cpeterson
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
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
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.
The issue is reproducible on Firefox Beta 14.0b2 and Aurora 14.0a2 2012-05-21.
Keywords: qawanted
No longer blocks: 743468
blocking-fennec1.0: ? → +
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)
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)
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+
(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().
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?
Status: ASSIGNED → RESOLVED
Closed: 13 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+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: