Closed Bug 882866 Opened 9 years ago Closed 9 years ago

When correcting a word, previous letters and words seem to join on to the front of the word


(Firefox OS Graveyard :: Gaia::Keyboard, defect)

Not set


(blocking-b2g:leo+, firefox22 wontfix, firefox23 wontfix, firefox24 fixed, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

1.1 QE3 (26jun)
blocking-b2g leo+
Tracking Status
firefox22 --- wontfix
firefox23 --- wontfix
firefox24 --- fixed
b2g18 --- verified
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- fixed


(Reporter: djabber, Assigned: xyuan)



(5 files)

I've noticed when pressing backspace to go back and correct a mis-spelt a word, occasionally the auto-correct feature joins on previous letters and words to the word being corrected, even though they're separated by a space. This leads to auto-correct suggestions such as the following:

tha -> Ethan rtha ethane
im -> immune imm Inman
second -> andsecond
device -> devidvice
This might be a symptom of bug 882197. Can you still reproduce it on master?
I can reproduce it on master. This generally seems to happen when typing in text very fast. I think the issue might in fact be that characters that have been entered in are being somehow missed by the auto-correct feature. See the following attachments.
Attached image Missing letters
Attached image Missing letters
Attached image Missing letters
Attached image Missing letters
Thanks for these screenshots, Francis. 

I think I can reproduce what you're seeing (or something like it) with these steps:

1) Type "testin" and see that auto-correct is going to replace it with "testing".

2) Type two spaces very quickly. We get an autocorrect and a period.  But if I now backspace twice, and type space again (to autocorrect again) I get 'ttesting". This indicates that the latin im is confused about the current state of the input.  (I can reproduce this in both the MMS app that uses content editable and in the UI tests that use text area).

I noticed the other day that with the switch to using replaceSurroundingText(), the keyboard code in gecko seems to be re-activating the keyboard code in gaia after each word replacement. This happens asynchronously, and if the user is typing quickly, it might be possible for the gaia representation of input field content to get out of sync with the gecko reality, and this would cause auto-correct errors like the ones here seeing.

Yuan: do you know anything about this? Does calling replaceSurroundingText() end up re-activiting the keyboard somehow the way we would if the user tapped to move the cursor?

This seems bad, so I'm nominating it for Leo. I'm assigning the bug to myself so I don't forget about it. Yuan and Rudy: if either of you want to take this, please feel free.
Assignee: nobody → dflanagan
blocking-b2g: --- → leo?
Flags: needinfo?(xyuan)
(In reply to David Flanagan [:djf] from comment #7)
> Yuan: do you know anything about this? Does calling replaceSurroundingText()
> end up re-activiting the keyboard somehow the way we would if the user
> tapped to move the cursor?

Yes, you are right. mozKeyboard.replaceSurroundingText will sometime reactivate the keyboard. 

we added an observer to monitor all edit actions of the current input element. If the text of the element is changed by external actions, such as using JS to clear the content, we will get notification to make keyboard reactivate. When mozKeyboard.replaceSurroundingText is called to replace text, two edit actions will be observed: removal of the old text and insertion of the new text. The first action is ignored, while the second is treated as external actions mistakenly. 

I'll make a patch to resolve the issue of mozKeyboard.replaceSurroundingText, but I'm sure it could fix this bug.
Flags: needinfo?(xyuan)
The patch ignored all edit actions caused by mozKeyboard to avoid reactivating the keyboard mistakenly.
Attachment #764744 - Flags: review?(fabrice)
blocking-b2g: leo? → leo+
Comment on attachment 764744 [details] [diff] [review]
Ignore EditAction notification caused by mozKeyboard.replaceSurroundingText. r=fabrice

Review of attachment 764744 [details] [diff] [review]:

Fabrice is on vaction, I think, so I'm stealing this review.

The patch is straightforward. The code is clean. I've tested it and it fixes the bug described in comment 7.

I've never been able to type quickly enough to reproduce the exact symptoms that the orignal reporter described. But this patch does fix a major bug, so we should land it ASAP and ask the reporter to verify.

::: b2g/chrome/content/forms.js
@@ +275,2 @@
>        return;
>      }

Even with this code, we'll still have a race condition if the user is typing at the same time that a script modifies the text field content. In that case, the Latin IM may get confused and do bad auto-corrections.  

I can't think of any way to fix this with our current architecture, and it seems quite unlikely in practice.

@@ +376,5 @@
>          }.bind(this), 0);
>          break;
>        case "keyup":
> +        this._ignoreEditActionOnce = false;

If a keyup event arrives before the edit action occurs, then we won't ignore the edit action.

But maybe this cannot ever occur. If it can, it is a different race condition than the one this patch is fixing.
Attachment #764744 - Flags: review?(fabrice) → review+
Setting checkin-needed, and re-assigning to Yuan, since he's the one who wrote the patch.
Assignee: dflanagan → xyuan
Keywords: checkin-needed
This is a b2g patch, so checkin should be to the birch branch, of course.
Closed: 9 years ago
Resolution: --- → FIXED
can you upload full source about b2g/chrome/content/forms.js
Verified fixed on Leo device
Build ID: 20130807071207
Gaia: 60ca81600a080dae33058b0692ecaa213556c926
Platform Version: 18.1

Auto correct is now functioning efficiently
You need to log in before you can comment on or make changes to this bug.