Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

beforeinput and compositionupdate event order #354

Open
zcorpan opened this issue Jul 5, 2023 · 8 comments
Open

beforeinput and compositionupdate event order #354

zcorpan opened this issue Jul 5, 2023 · 8 comments

Comments

@zcorpan
Copy link
Member

zcorpan commented Jul 5, 2023

About [beforeinput] vs. compositionupdate, I'm surprised at the event order in Chrome. compositionupdate represents the composition string change, it does not depend on editing content from UI Events spec point of view. Therefore, I think that the builtin editor should handle compositionupdate and dispatch beforeinput and input (as its default action). Therefore, I think that compositionupdate -> beforeinput -> textInput -> input is reasonable.

Originally posted by @masayuki-nakano in #353 (comment)

The event order of typing ´a in an input field in Chrome is (see #353 (comment)):

  • keydown
  • compositionstart
  • beforeinput
  • compositionupdate
  • input
  • keyup
  • keydown
  • beforeinput
  • compositionupdate
  • textInput
  • input
  • compositionend
  • keyup

cc @chrishtr
@zcorpan zcorpan mentioned this issue Jul 5, 2023
Closed
@chrishtr
Copy link

chrishtr commented Aug 23, 2023 •
edited
Loading

So you want the order to be this right? (highlighted move)

keydown
compositionstart
compositionupdate
beforeinput
input
keyup
keydown
compositionupdate
beforeinput
textInput
input
compositionend
keyup

@garykac
Copy link
Member

garykac commented Aug 24, 2023

Nit: I think both compositionupdates would move before their corresponding beforeinputs.

@johanneswilm johanneswilm mentioned this issue Sep 1, 2023
Closed
@garykac
Copy link
Member

garykac commented Sep 14, 2023

It looks like we have agreement here on the order, so I'll be updating the spec.

@chrishtr
Copy link

Please don't update it yet. Chromium has not confirmed if this is web compatible.

@garykac
Copy link
Member

garykac commented Sep 18, 2023

My understanding (from #353 (comment)) is that Firefox and Safari already ship with this event ordering.

What is the work involved in checking web compatablity? Do we have a tracking bug for this?

@chrishtr
Copy link

Tracked here.

aarongable pushed a commit to chromium/chromium that referenced this issue Sep 22, 2023
@siliu1
[IME]Fire compositionupdate event before beforeinput event.
65dc007 
Per w3c/uievents#354, we should fire compositionupdate event before the beforeinput event so that the event order is the same as other browsers such as Safari and Firefox.


Bug: 1484762
Change-Id: I4462d2cc214d9e7f9a40c83d54d553d3379fab48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4878553
Reviewed-by: Xianzhu Wang <[email protected]>
Reviewed-by: Hidehiko Abe <[email protected]>
Commit-Queue: Siye Liu <[email protected]>
Reviewed-by: Darren Shen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1200044}
@zcorpan
Copy link
Member Author

zcorpan commented Nov 23, 2023

@chrishtr the chromium fix has now reached Stable (assuming it shipped in 119). I searched for "beforeupdate" and "compositionupdate" in https://bugs.chromium.org but didn't find relevant recently filed bugs.

@chrishtr
Copy link

chrishtr commented Dec 8, 2023

@chrishtr the chromium fix has now reached Stable (assuming it shipped in 119). I searched for "beforeupdate" and "compositionupdate" in https://bugs.chromium.org but didn't find relevant recently filed bugs.

Agreed! I double checked and couldn't find any issues either. Looks like the change was web compatible.

@garykac could you update the spec? Thanks!

aarongable pushed a commit to chromium/chromium that referenced this issue Dec 8, 2023
@siliu1
Remove CompositionUpdateBeforeBeforeInput runtime feature flag.
573d38d 
The feature flag was introduced in crrev.com/1200044 to guard the new
behavior of compositionupdate event firing order. The new behavior was
shipped in 119. There is no issue filed against the behavior since then
and is confirmed in
w3c/uievents#354 (comment).

Change-Id: Ib90621f8fd1236d9cadd08782150245c19c46971
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5106828
Reviewed-by: Xianzhu Wang <[email protected]>
Commit-Queue: Siye Liu <[email protected]>
Reviewed-by: Sanket Joshi <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1235290}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@zcorpan @garykac @chrishtr