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)
Tracking
(firefox14 verified, firefox15 verified, firefox16 verified, firefox17 verified, blocking-fennec1.0 +)
VERIFIED
FIXED
Firefox 15
People
(Reporter: AdrianT, Assigned: cpeterson)
References
()
Details
(Keywords: regression, Whiteboard: HKB)
Attachments
(2 files)
4.96 KB,
patch
|
blassey
:
review+
blassey
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
4.22 KB,
patch
|
blassey
:
review+
|
Details | Diff | Splinter Review |
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•13 years ago
|
status-firefox14:
--- → affected
status-firefox15:
--- → affected
Reporter | ||
Updated•13 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•13 years ago
|
Assignee: nobody → cpeterson
Assignee | ||
Comment 1•13 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.
Keywords: regressionwindow-wanted
Whiteboard: HKB
Reporter | ||
Comment 2•13 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
Keywords: regressionwindow-wanted → regression
Assignee | ||
Comment 3•13 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: --- → ?
Comment 4•13 years ago
|
||
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
Comment 5•13 years ago
|
||
(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•13 years ago
|
||
The issue is reproducible on Firefox Beta 14.0b2 and Aurora 14.0a2 2012-05-21.
Keywords: qawanted
Assignee | ||
Comment 7•13 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•13 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•13 years ago
|
Status: NEW → ASSIGNED
Comment 9•13 years ago
|
||
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+
Updated•13 years ago
|
Attachment #628977 -
Flags: review?(blassey.bugs) → review+
Assignee | ||
Comment 10•13 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 11•13 years ago
|
||
Assignee | ||
Comment 12•13 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•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9a6a5cc5a4a5
https://hg.mozilla.org/mozilla-central/rev/180b78b6cec4
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
Updated•13 years ago
|
status-firefox15:
affected → ---
Comment 14•13 years ago
|
||
Comment on attachment 628973 [details] [diff] [review]
part-1-commit-on-reinitialization.patch
[Triage Comment]
Attachment #628973 -
Flags: approval-mozilla-aurora? → approval-mozilla-beta+
Assignee | ||
Comment 15•13 years ago
|
||
status-firefox15:
--- → fixed
Updated•13 years ago
|
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•