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

Status: RESOLVED FIXED

(GeckoView :: General, defect, P2)

Description

[naktinis]

Attachment: Screenrecorder-2019-08-05-10-13-08-916.mp4

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".

Comment 1

[Diana Rus]

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.

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

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

Comment 3

[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).

Comment 4

[Chris Peterson [:cpeterson]]

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?

Comment 5

[Makoto Kato [:m_kato]]

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)

Comment 6

[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.

Comment 7

[Makoto Kato [:m_kato]]

(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).

Comment 8

[Makoto Kato [:m_kato]]

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

Comment 9

[Makoto Kato [:m_kato]]

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

Comment 10

[Makoto Kato [:m_kato]]

Attachment: mochitest

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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).

Comment 12

[Makoto Kato [:m_kato]]

Attachment: Bug 1571375 - Don't remove white space when committing composition. r=masayuki

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.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4dd3945f8b0a
Don't remove white space when committing composition. r=masayuki

Comment 14

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/4dd3945f8b0a

Comment 15

[Chris Peterson [:cpeterson]]

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.

Comment 16

[will69]

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

Comment 17

[Makoto Kato [:m_kato]]

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:

Comment 18

[Makoto Kato [:m_kato]]

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

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/412870943921

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/bf8b0830cda8

Comment 22

[Laurentiu Apahidean]

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.