Closed Bug 1571375 Opened 4 months ago Closed 3 months ago

Typing space followed by two characters in contenteditable span replaces the space with first character

Categories

(GeckoView :: General, defect, P2)

ARM
Android

Tracking

(firefox-esr60 unaffected, firefox-esr68 verified, firefox69 wontfix, firefox70 fixed, firefox71 fixed)

RESOLVED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- verified
firefox69 --- wontfix
firefox70 --- fixed
firefox71 --- fixed

People

(Reporter: naktinis, Assigned: m_kato)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

Typing words into a <span contenteditable></span> element. Pressed the space key followed by a couple of character keys.

Actual results:

Say, I'm typing "Hello world". As I type "world", after pressing "o" <span> content becomes "Hellowwo". I could also reproduce this on a different Android phone with Firefox.

Expected results:

The content should have been "Hello wo".

Hi, I reproduce the issue on Fennec beta 68.1b4, Fennec Nightly 68.1a1(2019-08-04), Fennec 68.0 and Firefox 70 with both Samsung Galaxy S8+ (Android 8.0.0) and with Sony Xperia Z5.
I will mark the ticket as new.
Note: On Chrome this outcome does not occur.

Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Unspecified → Android
Hardware: Unspecified → ARM

What is the behavior on Firefox Preview and/or Reference Browser?

Flags: needinfo?(diana.rus)

Hi,
I reproduce the issue with both Firefox Preview Browser 1.1.0 (Build #12042112) and Reference Browser 1.0.1931 (Build #12151213 GV: 70.0a1 - 20190731215544) - (both downloaded from Play Store) with devices Sony Xperia Z5 (Android 7.0) and Samsung Galaxy S8+ (Android 8.0).

Flags: needinfo?(diana.rus)
Product: Firefox for Android → GeckoView
Target Milestone: --- → mozilla71
Version: Firefox 68 → Trunk

Hi Makoto, this text input bug affects both Fennec and Fenix/GeckoView. Kevin says multiple similar bugs have been reported by Fenix users, so this sounds like a common problem. Do you think this is a high or low priority bug?

Rank: 29
Flags: needinfo?(m_kato)
Priority: -- → P2

I guess that this depends on keyboard, and for priority, I want to know some information.

Diana, I would like to know more information for your environment.

devices Sony Xperia Z5 (Android 7.0) and Samsung Galaxy S8+ (Android 8.0).

What's your keyboard? Samsung Keyboard? And since I have land bug 1563640 last month, could you test this with the latest Fenix (Fenix's Nightly is Build #12481253 with GV71)

Flags: needinfo?(m_kato) → needinfo?(diana.rus)

Hi, verified this with Fenix Nightly Build ID: 12490612 with Samsung Galaxy Note 9 (Android 8.1.0) and Samsung Galaxy S8+ (Android 8.0.0), both with Samsung Keyboard set as default and the issue is still reproduce-able.

Flags: needinfo?(diana.rus)

(In reply to Diana Rus from comment #6)

Hi, verified this with Fenix Nightly Build ID: 12490612 with Samsung Galaxy Note 9 (Android 8.1.0) and Samsung Galaxy S8+ (Android 8.0.0), both with Samsung Keyboard set as default and the issue is still reproduce-able.

Thanks, Diana.

(In reply to Chris Peterson [:cpeterson] from comment #4)

Hi Makoto, this text input bug affects both Fennec and Fenix/GeckoView. Kevin says multiple similar bugs have been reported by Fenix users, so this sounds like a common problem. Do you think this is a high or low priority bug?

I can reproduce this with GBoard too. But this doesn't seem to occurs if using <div contenteditable>, not <span>. I think that this is high priority although this is only <span>. (Some sites use <span> for contentedtiable.)

But after some import bugs in Q3 are finished, I will handle this (in Q3 or Q4).

(maybe, since internal <br> element isn't inserted to <span> element, this will occur.)

WSRunObject::InsertText will remove whitepsace unfortunately since beforeRun->mType & WSType::trailingWS is true. This is editor bug.

Assignee: nobody → m_kato
Keywords: regression
Regressed by: 1530649
Attached patch mochitestSplinter Review

Ah, WSRunObject::GetRuns() assumes that mStartNode/mStartOffset and mEndNode/mEndOffset are always same (i.e., collapsed). If I had much time next week, I'd take this (but I already have 2 important bugs).

This is regression by bug 1530649.

After landing bug 1530649, we try to scan end point of replacement text. But
in this bug's situation, afterRun becomes same as current ws run by landing
bug 1530649. To get white space type of next of replacement end, we have to
scan around end point again.

Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/4dd3945f8b0a
Don't remove white space when committing composition. r=masayuki
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED

Makoto, should we uplift your composition fix to Fennec ESR 68 or GeckoView 70 Beta (for Fenix)? This regression from bug 1530649 landed 68, so it's the current behavior of both Fennec and Fenix. Since your fix modifies Gecko's libeditor code used on all platforms, uplifting might be risky. OTOH, your fix will never reach Fennec users unless we uplift to ESR 68.

Flags: needinfo?(m_kato)

I am affected by this problem right now, so I sincerely hope you do decide to uplift to Fennec ESR 68.

Comment on attachment 9092887 [details]
Bug 1571375 - Don't remove white space when committing composition. r=masayuki

Beta/Release Uplift Approval Request

  • User impact if declined: When editing HTML using GBoard, user may not be able to input multiple word on some contents.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is regression by bug 1530649. So we fix both issue by this fix. Bug 1530649's fix mistakes about current insertion point range of HTML element when using Android's IME.
  • String changes made/needed:
Flags: needinfo?(m_kato)
Attachment #9092887 - Flags: approval-mozilla-beta?

Comment on attachment 9092887 [details]
Bug 1571375 - Don't remove white space when committing composition. r=masayuki

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: This issue occurs on Fennec 68.x. And Fenix isn't stable yet, so we need fix on 68.x for release channel.
  • User impact if declined: When editing HTML using GBoard, user may not be able to input multiple word on some contents.
  • Fix Landed on Version: 71
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is regression by bug 1530649. So we fix both issue by this fix. Bug 1530649's fix mistakes about current insertion point range of HTML element when using Android's IME.
  • String or UUID changes made by this patch: no
Attachment #9092887 - Flags: approval-mozilla-esr68?

Comment on attachment 9092887 [details]
Bug 1571375 - Don't remove white space when committing composition. r=masayuki

Fixes an IME issue. Approved for GV70 and Fennec 68.2b4.

Attachment #9092887 - Flags: approval-mozilla-esr68?
Attachment #9092887 - Flags: approval-mozilla-esr68+
Attachment #9092887 - Flags: approval-mozilla-beta?
Attachment #9092887 - Flags: approval-mozilla-beta+

I tried to reproduce the issue on Beta 68.2b4 on a Samsung Galaxy S8+ (Android 8.0.0) and Pixel 3 XL (Android 9) but was unable to, I will mark this issue accordingly.

You need to log in before you can comment on or make changes to this bug.