Duplicated text when typing in a website's textbox

RESOLVED FIXED in Firefox 62

Status

defect
P1
normal
RESOLVED FIXED
Last year
7 months ago

People

(Reporter: ekager, Assigned: mbrubeck)

Tracking

unspecified
mozilla65
Dependency tree / graph

Firefox Tracking Flags

(geckoview64blocking fixed, firefox-esr52 wontfix, firefox-esr60 wontfix, firefox61 wontfix, firefox62 verified, firefox63 wontfix, firefox64 wontfix, firefox65 fixed)

Details

(Whiteboard: [geckoview:klar:p1])

Attachments

(3 attachments)

Reported on Slack - "When typing into the Google search box on google.com, each previous keypress gets repeated. So, typing "Berlin" actually gives "BBeBerBerlBerliBerlin". Is this already a known issue? I am on recent Nightly." 

Can also reproduce on github search, etc and with GeckoView Example app.
Priority: -- → P1
Whiteboard: [geckoview:klar]
Emily, GV engineers can't reproduce this behavior. What Android device and OS version are you using? Are you using a third-party keyboard app?
Flags: needinfo?(ekager)
My device is a HTC U11 with Android 8.0. Keyboard ist TouchPal - that's the default keyboard on current HTC devices. According to another user on GitHub the problem also exists with the LG G6 and Android 8.0.
I can reproduce on emulator with x86 builds on Nexus 4 API22 and have attached a gif of the behavior.
Flags: needinfo?(ekager)
We've had issues like this in the past. I see this on youtube.com when typing in their search bar on a Galaxy S8 US w/ Samsung keyboard w/ Focus Geckoview.
[geckoview:klar:p1] because this is a Focus+GV blocker.
Whiteboard: [geckoview:klar] → [geckoview:klar:p1]
Comment on attachment 8992052 [details]
Bug 1470786 - 1. Support async text changes from replacing text;

https://reviewboard.mozilla.org/r/256970/#review264124
Attachment #8992052 - Flags: review?(esawin) → review+
Comment on attachment 8992053 [details]
Bug 1470786 - 2. Fix a text sync issue;

https://reviewboard.mozilla.org/r/256972/#review264126
Attachment #8992053 - Flags: review?(esawin) → review+
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d4c93d40e9aa
1. Support async text changes from replacing text; r=esawin
https://hg.mozilla.org/integration/autoland/rev/82da16c50e25
2. Fix a text sync issue; r=esawin
https://hg.mozilla.org/mozilla-central/rev/d4c93d40e9aa
https://hg.mozilla.org/mozilla-central/rev/82da16c50e25
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Comment on attachment 8992052 [details]
Bug 1470786 - 1. Support async text changes from replacing text;

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: Text input in general is buggy under e10s GeckoView
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Slightly
[Why is the change risky/not risky?]: The code fixes text input behavior for GeckoView (e10s) but should have no impact on Fennec (under non-e10s) 
[String changes made/needed]: None
Attachment #8992052 - Flags: approval-mozilla-beta?
Comment on attachment 8992052 [details]
Bug 1470786 - 1. Support async text changes from replacing text;

Geckoview fix, let's uplift for beta 11.
Attachment #8992052 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8992053 [details]
Bug 1470786 - 2. Fix a text sync issue;

[Triage Comment]
Attachment #8992053 - Flags: approval-mozilla-beta+
Depends on: 1477469
Tested with HTC Desire 820(Android 6.0.1) and HTC 10(Android 8.0.0) on latest Beta build (62.0b11) and couldn't reproduce the issue.
Verified as fixed on latest Nightly build (08/16).
Device: OnePlus 5T(Android 8.1.0).
Status: RESOLVED → VERIFIED
See Also: → 1490391
Depends on: 1490391
See Also: 1490391
Reopening because the bugfix has been reverted in bug 1505161 and makes Firefox Focus unusable again. See https://github.com/mozilla-mobile/focus-android/issues/3965.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Assignee: jimnchen+bmo → nobody
See Also: → 1508552
See Also: 1508552
Matt said he'd take this - thanks!
Assignee: nobody → mbrubeck
Current status:

- This fix was backed out of 63 (Release) and 64 (Beta), so those branches are affected (again).

- On 65 (Nightly), bug 1490391 fixed the regression instead, so no back-out was needed.  65 is unaffacted.

In bug 1490391 comment 30, jchen said the release/beta backout should include "an exception for Focus."  Chris, can we re-land this fix on a GV-only or Focus-only branch for 63 and/or 64?  Or would we need to add a conditional in the code, so that the same source code works for both GV and Fennec?

Alternately, we could reconsider bug 1490391 for uplift.
(In reply to Matt Brubeck (:mbrubeck) from comment #22)
>
> Alternately, we could reconsider bug 1490391 for uplift.

Although I am investigating bug 1508552, I believe that landing this and bug 1490391 to beta is better.
(In reply to Makoto Kato [:m_kato] from comment #23)
> (In reply to Matt Brubeck (:mbrubeck) from comment #22)
> >
> > Alternately, we could reconsider bug 1490391 for uplift.
> 
> Although I am investigating bug 1508552, I believe that landing this and bug
> 1490391 to beta is better.

Makoto, if we uplift a fix for bug 1508552 and bug 1490391 to 64 Beta, will we still need to limit the bug 1490391 fix to just GV or Focus?

If so, I can work with Relman to create a new GV-only branch of 64 Beta. Creating a beta branch will be a hassle. Alternately, since 64 Release is only two weeks away (December 10), we could wait and create the GECKOVIEW_64_RELBRANCH (in mozilla-release) that we will probably need anyways for Focus.

What do you recommend?

(In reply to Matt Brubeck (:mbrubeck) from comment #22)
> In bug 1490391 comment 30, jchen said the release/beta backout should
> include "an exception for Focus."  Chris, can we re-land this fix on a
> GV-only or Focus-only branch for 63 and/or 64?  Or would we need to add a
> conditional in the code, so that the same source code works for both GV and
> Fennec?
Flags: needinfo?(cpeterson) → needinfo?(m_kato)
(In reply to Chris Peterson [:cpeterson] from comment #24)
> (In reply to Makoto Kato [:m_kato] from comment #23)
> > (In reply to Matt Brubeck (:mbrubeck) from comment #22)
> > >
> > > Alternately, we could reconsider bug 1490391 for uplift.
> > 
> > Although I am investigating bug 1508552, I believe that landing this and bug
> > 1490391 to beta is better.
> 
> Makoto, if we uplift a fix for bug 1508552 and bug 1490391 to 64 Beta, will
> we still need to limit the bug 1490391 fix to just GV or Focus?

Yes and No.  Hmm, I need test it for GV.  Now although I test bug 1493428's situation on GV 62 (Focus 7.0. This bug's fix is landed), it has another problem. (Some IMEs works well, but POBOX (bug 1493428) doesn't work well.  It seems to be just bug 1490391)

So I will answer it tomorrow again after testing it.  (inc. I will test whether bug 1490391 occurs on GV 64 after re-landing this fix)

> If so, I can work with Relman to create a new GV-only branch of 64 Beta.
> Creating a beta branch will be a hassle. Alternately, since 64 Release is
> only two weeks away (December 10), we could wait and create the
> GECKOVIEW_64_RELBRANCH (in mozilla-release) that we will probably need
> anyways for Focus.

Since 64 release is within 2 weeks, GV only branch is better.  After GV branch is created, we should re-land this to it since this bug is fixed at least.

BTW, what release date of Firefox Focus 8?
(In reply to Makoto Kato [:m_kato] from comment #25)
> Since 64 release is within 2 weeks, GV only branch is better.  After GV
> branch is created, we should re-land this to it since this bug is fixed at
> least.
> 
> BTW, what release date of Firefox Focus 8?

Focus 8's release date is December 17. GV 64's release date is December 11. The Focus team will probably want a fix in GV 64 Beta before December 11 so they have more beta test time for GV 64 in Focus 8 Beta.

I will ask Releng about making a GV-only beta branch.
Makoto, if these IME bugs affect Focus+GV but not Fennec, will they affect Custom Tabs that use Fennec's GV? If so, then maybe we do need a run-time check for an IME workaround?
(In reply to Chris Peterson [:cpeterson] from comment #27)
> Makoto, if these IME bugs affect Focus+GV but not Fennec, will they affect
> Custom Tabs that use Fennec's GV? If so, then maybe we do need a run-time
> check for an IME workaround?

This issue depends on e10s.  Custom tab doesn't use e10s. (https://searchfox.org/mozilla-central/rev/0859e6b10fb901875c80de8f8fc33cbb77b2505e/mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java#127)  Focus will use e10s.
(In reply to Makoto Kato [:m_kato] from comment #25)
> (In reply to Chris Peterson [:cpeterson] from comment #24)
> > (In reply to Makoto Kato [:m_kato] from comment #23)
> > > (In reply to Matt Brubeck (:mbrubeck) from comment #22)
> > > >
> > > > Alternately, we could reconsider bug 1490391 for uplift.
> > > 
> > > Although I am investigating bug 1508552, I believe that landing this and bug
> > > 1490391 to beta is better.
> > 
> > Makoto, if we uplift a fix for bug 1508552 and bug 1490391 to 64 Beta, will
> > we still need to limit the bug 1490391 fix to just GV or Focus?
> 
> Yes and No.  Hmm, I need test it for GV.  Now although I test bug 1493428's
> situation on GV 62 (Focus 7.0. This bug's fix is landed), it has another
> problem. (Some IMEs works well, but POBOX (bug 1493428) doesn't work well. 
> It seems to be just bug 1490391)
> 
> So I will answer it tomorrow again after testing it.  (inc. I will test
> whether bug 1490391 occurs on GV 64 after re-landing this fix)

Today, I test some fixes and environments.

bug 1490391 is for non-e10s.  Even if landing bug 1490391, it isn't unrelated to Focus, so it is unnecessary for Focus  So if we have GV only branch, it is OK to re-land this bug (bug 1470786) only.

Although some input methods still have bugs that composing string is committed unexpectedly, it isn't fixed even if the latest GV 65.  So I will investigate it by bug 1510527.
Flags: needinfo?(m_kato)
Blocks: 1508552
In anticipation of the creation of the GECKOVIEW_64_RELBRANCH next week, I am moving this bug from status-firefox64=affected to status-geckoview64=affected.
Matt or Makoto, do these patches need to be uplifted to the GV 64 relbranch? If so, can you please request approval-mozilla-geckoview64 uplift approval so RyanVM knows land these patches when he creates the GV 64 relbranch on December 3?
Flags: needinfo?(mbrubeck)
Flags: needinfo?(m_kato)
(In reply to Chris Peterson [:cpeterson] from comment #31)
> Matt or Makoto, do these patches need to be uplifted to the GV 64 relbranch?
> If so, can you please request approval-mozilla-geckoview64 uplift approval
> so RyanVM knows land these patches when he creates the GV 64 relbranch on
> December 3?

I will add revert patch for uplifting to GV64 relbranch.
Flags: needinfo?(m_kato)
mbrubeck, do you already have revert patch for this?   Since I need sync 64 release branch from https://hg.mozilla.org/releases/mozilla-release.  If no, I will sync tree, then I will create a patch.
The original patch here applies cleanly to the 64 release branch, using these commands:

hg pull release
hg update GECKOVIEW_64_RELBRANCH
hg import https://hg.mozilla.org/releases/mozilla-beta/rev/7e6f51f7fca4

I'll request approval on the existing patch.
Flags: needinfo?(mbrubeck)
Comment on attachment 8992052 [details]
Bug 1470786 - 1. Support async text changes from replacing text;

[GeckoView Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for consideration: This fixes a critical bug in Android apps that use GeckoView.

User impact if declined: Users of Firefox Focus are unable to enter text in web pages using certain devices or keyboards.

Fix Landed on Version: 65

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Risk is minimal because this is landing on a GeckoView-only branch.  This patch has been in Nightly 65 for several months, and previously landed in Beta 64 for several weeks.  In both branches, it did not cause any problems in GeckoView apps; it was backed out only as a low-risk fix for a non-GeckoView regression.

String or UUID changes made by this patch: n/a
Attachment #8992052 - Flags: approval-mozilla-geckoview64?
Approving for GV64, calling this fixed for 65+ by bug 1490391.
Status: REOPENED → RESOLVED
Closed: Last year8 months ago
Resolution: --- → FIXED
Target Milestone: Firefox 63 → Firefox 65
Attachment #8992052 - Flags: approval-mozilla-geckoview64? → approval-mozilla-geckoview64+
Duplicate of this bug: 1508552
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 65 → mozilla65
You need to log in before you can comment on or make changes to this bug.