Closed Bug 1530649 Opened 2 years ago Closed 2 years ago

Typing Vietnamese in Google Hangout on macOS get spaces automatically removed

Categories

(Core :: DOM: Editor, defect, P1)

65 Branch
x86_64
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: tule.hut, Assigned: masayuki)

References

(Depends on 1 open bug)

Details

(Whiteboard: [webcompat])

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.119 Safari/537.36

Steps to reproduce:

I'm using macOS Mojave 10.14.3 (18D109)

  • Step 1: switch to Vietnamese > Telex or Simple Telex input source.
  • Step 2: open Firefox and visit gmail.com or hangout.google.com.
  • Step 3: open a chat box and start entering the following message: thank you

All other websites that I have used work normally.

Actual results:

thankyou
(the space was remove right after I typed "y" following the space)

Expected results:

thank you

Hi reporter,

Thank you for taking the time to add this report. However, I was not able to reproduce it using the latest Firefox Release and Nightly builds on Mac OS 10.14.

Could you please retest this using the latest FF release and latest Nightly build (https://nightly.mozilla.org/) and report back the results? When doing this, please use a new clean Firefox profile, maybe even safe mode, to eliminate custom settings as a possible cause (https://goo.gl/PNe90E).

I'm going to add this to Core: Widget:Cocoa component for a more advised opinion (if required), but if this is not the proper component, feel free to change it to a more appropriate one.

Component: Untriaged → Widget: Cocoa
Flags: needinfo?(tule.hut)
Product: Firefox → Core
Priority: -- → P3
Flags: webcompat?
Whiteboard: [webcompat]

Looks like that this is a bug of editor. Once <br> element is created into the <div contenteditable>, you won't see this bug. Perhaps, editor trims last whitespace which was inserted by a composition.

Status: UNCONFIRMED → NEW
Component: Widget: Cocoa → Editor
Ever confirmed: true
Flags: needinfo?(tule.hut)
OS: Unspecified → macOS
Hardware: Unspecified → x86_64

Perhaps, I got it.

Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Priority: P3 → P2

Unfortunately, this bug is really long standing bug since first IME support in our editor. So, coming patch becomes big and risky. Therefore, we cannot fix this in the beta channel...

If insertion string ends with ASCII whitespace and there is no following
content in the block, HTMLEditRules::AdjustWhitespaces() needs to insert
<br> element. It's called only by HTMLEditRules::AfterEditInner() and
that does only simple things with WSRunObject. Therefore, this moves the
code into AfterEditInner().

For making it adjust the whitespaces, HTMLEditRules::WillInsertText() needs
to notify AfterEditInner() of dirty range with mDocChangeRange. Therefore,
this patch makes it set mDocChangeRange manually after inserting composition
string.

On the other hand, there is another bug. WSRunObject was designed to treat
only inserting text for WSRunObject::InsertText(). I.e., not designed to
treat replacing existing composition string with new string. Therefore,
WSRunObject::InsertText() adjusts whitespaces only around start of
composition string. Therefore, if composition string ends with an ASCII
whitespace, it's not replaced with NBSP and that causes:

  • failing WSRunObject::AdjustWhitespaces() inserts <br> element at
    AfterEditInner() of committing composition.
  • then, next composition's first WSRunObject::InsertText() removes the
    last whitespace due to not followed by <br> nor any other content.
    Therefore, this patch makes WSRunObject takes 2 DOM points to be able to
    treat replaced range.

In strictly speaking, the latter change require more changes and tests for
supporting replacement with any other methods. However, it's risky and out
of scope of this bug.

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/70d5e0b1c0f8
Improve composition string handling which ends with whitespaces r=m_kato
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Regressions: 1571375
See Also: → 1584901
Regressions: 1623913
Depends on: 1649114
You need to log in before you can comment on or make changes to this bug.