Closed Bug 1722229 Opened 3 years ago Closed 3 years ago

Bug when entering Japanese mode in facebook comments

Categories

(Core :: DOM: Events, defect, P2)

Firefox 90
Desktop
Windows 10
defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr78 --- unaffected
firefox90 --- fixed
firefox91 + fixed
firefox92 --- fixed

People

(Reporter: akihiko.kigure, Assigned: masayuki)

References

(Blocks 1 open bug, Regression, )

Details

(Keywords: inputmethod, regression)

Attachments

(3 files, 1 obsolete file)

Attached file bugzzila-facebook.zip

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:90.0) Gecko/20100101 Firefox/90.0

Steps to reproduce:

When entering Japanese mode in a facebook comment, the entered characters are overwritten.

Occurs only on Windows10 Firefox.
Does not occur on macOS (Big sur).
Linux unconfirmed.

Windows Build

Actual results:

When you enter a Facebook comment in Japanese mode, the entered characters are overwritten and the entered characters disappear.

It does not occur when posting a new post, but only when an additional comment is added.

Expected results:

If you enter a Facebook comment in Japanese mode, the entered characters will not be overwritten and the input will continue.

Component: Untriaged → DOM: Editor
Keywords: inputmethod
Product: Firefox → Core

I can also reproduce the issue in Nightly90.0a1 Windows10 and MS-IME.

Status: UNCONFIRMED → NEW
Ever confirmed: true

IME is Google Japanse Input.
Windows Detail version and Google Japanse Input was see new attachment zip file.

[Tracking Requested - why for this release]: Affects the next ESR91. Affects Japanese IME users on Windows.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c30ee90cac6ae40b8da6b373a9950527e04caaf7&tochange=55d1325436c164d91a31bbc65c809bbaa66a8e21

Has Regression Range: --- → yes
Has STR: --- → yes
OS: Unspecified → Windows 10
Regressed by: 1609291
Hardware: Unspecified → Desktop
Component: DOM: Editor → DOM: Events
Keywords: regression
Blocks: 1665530

Disabling beforeinput indeed fixes the odd behavior. I have no idea what caused the difference between the browsers. As far as the quick look of the source code, beforeinput event listener works with selection and keyboard event's information. Something around them may cause the web-compat, but it's hard to debugging. Karl-san, could you debug it deeper or contact Facebook developers?

Severity: -- → S2
Flags: needinfo?(kdubost)
Priority: -- → P2

Even more puzzling is that it is happening on Windows and not on macos according to the initial Comment.
I will contact facebook developers to see if we can accelerate a fix instead of trying to decipher minified code.

  1. login to facebook
  2. Go to a post
  3. select Japanese input mode
  4. Type a comment.

Expected:
Characters are added.

Actual:
Characters disappear

Something which seems close in the code in
https://static.xx.fbcdn.net/rsrc.php/v3i_Xf4/y3/l/en_US/RT7W6KadETfhs75dmY_Sz8DwylBnwRdAODwrqcXGFQa4YHGXgumDdFtSBCTUfZSzpFdjCmUluTxHvvbkYmD9fa8Ka7GjLFHmC12lAMrbbZs-tczEqFXy0Vt1AdWa79-Ji_rppRD6kqD4tMB7pxNRj2vAEC7zfyeyCoQLwuzhboZUjzNMwBb93xf-F3bUqJ7fjX5xUL.js?_nc_x=jsjp5qVeY-p

var v;
if (Bc)
  b: {
    switch (c) {
      case "compositionstart":
        var w = "onCompositionStart";
        break b;
      case "compositionend":
        w = "onCompositionEnd";
        break b;
      case "compositionupdate":
        w = "onCompositionUpdate";
        break b;
    }
    w = void 0;
  }
else
  Ic
    ? Gc(c, e) && (w = "onCompositionEnd")
    : "keydown" === c && 229 === e.keyCode && (w = "onCompositionStart");
w &&
  (Dc &&
    "ko" !== e.locale &&
    (Ic || "onCompositionStart" !== w
      ? "onCompositionEnd" === w && Ic && (v = bc())
      : ((Zb = i),
        ($b = "value" in Zb ? Zb.value : Zb.textContent),
        (Ic = !0))),
  (ca = Hd(f, w)),
  0 < ca.length &&
    ((w = new qc(w, c, null, e, i)),
    j.push({
      event: w,
      listeners: ca,
    }),
    v ? (w.data = v) : ((v = Hc(e)), null !== v && (w.data = v))));
(v = Cc ? Jc(c, e) : Kc(c, e)) &&
  ((f = Hd(f, "onBeforeInput")),
  0 < f.length &&
    ((i = new qc("onBeforeInput", "beforeinput", null, e, i)),
    j.push({
      event: i,
      listeners: f,
    }),
    (i.data = v)));

The value for c if c.startsWith('composition') in the code above for switch (c) is
when I type りんご
on macOS (where the issue doesn't reproduce):

12:25:51.805 compositionstart RT7W6KadETfhs75dmY_Sz8DwylBnwRdAODwrqcXGFQa4YHGXgumDdFtSBCTUfZSzpFdjCmUluTxHvvbkYmD9fa8Ka7GjLFHmC12lAMrbbZs-tczEqFXy0Vt1AdWa79-Ji_rppRD6kqD4tMB7pxNRj2vAEC7zfyeyCoQLwuzhboZUjzNMwBb93xf-F3bUqJ7fjX5xUL.js:20093
12:25:53.938 compositionupdate 5 RT7W6KadETfhs75dmY_Sz8DwylBnwRdAODwrqcXGFQa4YHGXgumDdFtSBCTUfZSzpFdjCmUluTxHvvbkYmD9fa8Ka7GjLFHmC12lAMrbbZs-tczEqFXy0Vt1AdWa79-Ji_rppRD6kqD4tMB7pxNRj2vAEC7zfyeyCoQLwuzhboZUjzNMwBb93xf-F3bUqJ7fjX5xUL.js:20093
12:25:56.550 compositionend RT7W6KadETfhs75dmY_Sz8DwylBnwRdAODwrqcXGFQa4YHGXgumDdFtSBCTUfZSzpFdjCmUluTxHvvbkYmD9fa8Ka7GjLFHmC12lAMrbbZs-tczEqFXy0Vt1AdWa79-Ji_rppRD6kqD4tMB7pxNRj2vAEC7zfyeyCoQLwuzhboZUjzNMwBb93xf-F3bUqJ7fjX5xUL.js:20093
Flags: needinfo?(kdubost)

Just to report that, this bug does not only affect Japanese input methods, but also Korean, and all other East Asian input methods, including those of Chinese (both Traditional and Simplified, including Hong Kong, Macau, Taiwan, and PRC), Taiwanese/Minnan, Hakka, Cantonese, etc. This affects the whole East Asia.

And all platforms seem to be affected, at least Windows and macOS, according to reports on Taiwan bulletin board.

I am tracking this bug for 91 but it's unlikely we can have a fix before our 91 ship date.

Let me correct it.
I confirmed that it also occurs on macOS.
OS, Firefox, Google Japanese Input version is attached

fwiw I contacted Facebook on our partner mailing-list but no answers yet.

I wrote a patch to block beforeinput event under specific domain. Do you want to use it temporarily? The risk is, if Facebook's some features require beforeinput in other places, it won't work instead of avoiding this issue.

Flags: needinfo?(jstutte)

Facebook's comment field is broken for IME users. If disabling beforeinput
from the pref, you can avoid this issue. However, according to the telemetry
(https://mzl.la/3f93UUz), disabling beforeinput cause regressions on other
web apps. Therefore, this patch adds a blocklist of preventing to dispatch
beforeinput events under specific domains and adds *.facebook.com into it.

Assignee: nobody → masayuki
Status: NEW → ASSIGNED

Ah, perhaps, I got what's going on.

While updating composition, Facebook sets normal selection with Selection.setBaseAndExtent. Then, our editor changes replacing range as expected. I keep investigating deeper...

Well, there are some bugs, not only one thing. So, I need more days to fix the differences from Blink...

Currently, I found these 3 issues:

  1. If selection is not collapse, updating composition string occurs after deleting the selected range.
  2. Once #1 is fixed, non-collapsed normal selection appears only on Gecko.
  3. Finally, if I use conversion of composition string, pressing Escape key multiple times to cancel composition does not delete the raw composition string.

#1 should be fixed in Gecko-side, but Chrome also has similar bug. So, Facebook runs different path on Gecko and Blink.

I'm still investigating #2 and #3, but basically, Facebook should stop modifying selection during composition and that must fix this bug.

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #16)

Currently, I found these 3 issues:

  1. If selection is not collapse, updating composition string occurs after deleting the selected range.
  2. Once #1 is fixed, non-collapsed normal selection appears only on Gecko.

Although, there is flickering issue of the normal selection, I can prevent this. However, they modify selection asynchronously from an event listener. So, we need to override the selection asynchronously too. That may cause breaking the other web apps and the other part of Facebook too. So, I'm not sure whether we should do it even with allowed-list which contains only Facebook's domain.

  1. Finally, if I use conversion of composition string, pressing Escape key multiple times to cancel composition does not delete the raw composition string.

With the fix of #2, this is partially fixed. But Facebook's undo manager records a lot of steps at conversion. So, it's anyway not useful.

So, currently, we don't have low risk fix for this bug since this is caused by Facebook's mistake...

jstutt: Could you decide whether we disable beforeinput only (but entire) in Facebook or keep waiting reply from Facebook?

Thanks for bringing this up. We've been working on a new text editor for Facebook (and we plan to open source it at some point this year) and there was a bug that was causing this. The issue was that we were applying the getTargetRanges() logic from the beforeinput event during insertCompositionText and then updating the selection to match this. We'll be deploying a fix today.

(In reply to Dominic Gannaway from comment #18)

Thanks for bringing this up. We've been working on a new text editor for Facebook (and we plan to open source it at some point this year) and there was a bug that was causing this. The issue was that we were applying the getTargetRanges() logic from the beforeinput event during insertCompositionText and then updating the selection to match this. We'll be deploying a fix today.

Thank you very much for the fix! If you know something different from Blink/WebKit around composition and using Web API, let us know. Aligning edge cases reduces potential risk of Gecko-only breakage in the future.

Flags: needinfo?(jstutte)

One thing we did notice was that MutationObservers seem to fire after the input and compositionend events on Firefox, whilst on Blink/Webkit they fire before. We use MutationObservers to "undo" DOM changes outside of that of our text editor, as we want to ensure the DOM matches the editor's internal view model state. That's where much of the "forking" has been and we have work arounds, but ideally it would be good for all browsers to have the same event/mutation observer sequencing.

Thank you. I confirmed that I don't reproduce this bug anymore with Firefox Nightly.

And I'll file a bug for comment 20 with a test.

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
Attachment #9233648 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: