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.

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

event order proposal when inserting replacement text such as text prediction, spell checker, etc. #152

Open
siliu1 opened this issue May 8, 2024 · 5 comments · May be fixed by w3c/uievents#378

Comments

@siliu1
Copy link

siliu1 commented May 8, 2024

writingsuggestions and spellcheck are attributes that control UA-provided writing assistance such as text prediction and spell checker. When user accepts a suggestion/correction, wrong order of events fired might confuse online editors which may produce unexpected results.

Here is a proposal that the order of events when inserting suggestions text:

Order: Eventtype inputType
1 beforeinput insertReplacementText
2 input insertReplacementText

The beforeinput event is cancelable. If it's cancelled, suggestion insertion should be aborted, and no input event is fired.

Composition events are not necessary therefore shouldn't be fired when inserting suggestions/corrections.

The proposal also applies to other suggestion insertions such as auto-correct, etc.

The issue is also posted in WHATWG: whatwg/html#10337

@johanneswilm
Copy link
Contributor

@siliu1 This makes sense. But is this not what the spec requires already (also for other input types)? Is there an implementation that does not follow this order?

@sanketj sanketj added the Agenda+ Queue this item for discussion at the next WG meeting label May 8, 2024
@siliu1
Copy link
Author

siliu1 commented May 8, 2024

@johanneswilm I think the focus of this proposal are:

  1. Make sure that the inputType of beforeinput and input events are insertReplacementText.
  2. Composition events should not be fired.

@johanneswilm
Copy link
Contributor

johanneswilm commented May 8, 2024

@siliu1 Point 1 sounds like a potential bug in specific browsers to me (which ones?). This is already specified in the spec.
On point 2 - I don't recall us having specified anywhere that certain input types should NOT have composition events. But I think that's because it should be obvious when no composition takes place and would probably be considered a bug already. Is this happening in some browser engines right now?

@css-meeting-bot
Copy link
Member

The Web Editing Working Group just discussed event order proposal when inserting replacement text such as text prediction, spell checker, etc..

The full IRC log of that discussion <sanketj_> Topic: event order proposal when inserting replacement text such as text prediction, spell checker, etc.
<sanketj_> github: https://github.com//issues/152
<dandclark> sanketj_: This came up when we were implementing writingSuggestions
<dandclark> ...: Edge & Safari, the set of events we fired caused compat issues
<dandclark> ...: Came out of discussion with Marcos where we should be more clear about where events are fired. That's what Siye is trying to do.
<dandclark> ...: Beforeinput, input should be fired
<dandclark> ...: Composition events should not be fired
<dandclark> ...: Problem is not unique to writingSuggestions. There's autocomplete...
<dandclark> ...: Can we have the same pattern for all of them?
<dandclark> johanneswilm: I don't understand what the issue is. The order is specified in UI events
<dandclark> ...: Doesn't specify that there's no composition, but specifies this is not part of a composition
<dandclark> ...: I take that as implying there will be no comp events
<dandclark> sanketj_: May be worth clearing up when a suggestion or correction is accepted, these rules are followed
<dandclark> ...: Not clear if that should be here or in HTML spec
<dandclark> johanneswilm: The order of these events is in UI events spec
<dandclark> ...: I thought that's pretty clear
<dandclark> sanketj_: Does it call out they should be fired when autocomplete, autofill is accepted?
<dandclark> johanneswilm: Probably written in different way.
<dandclark> ...: is part of the issue that Apple didn't implement which events should come out of this?
<dandclark> sanketj_: Easy for bugs to be introduced if this is not specified somewhere
<dandclark> smaug: I think it's clear in spec. *reads spec text*
<dandclark> sanketj_: Does it say these need to be fired when something is accepted?
<dandclark> snianu: When you type in android by default, it goes through composition. When you type in site, if browser supports spellcheck, then <missed>. If you type on virtual keyboard it also shows spelling suggestions. On android every keystroke comes through composition
<dandclark> johanneswilm: But that' smore of an androib bug
<dandclark> s/androib/android
<dandclark> ...: If you have keyboard and it's not communicating that it's spellchecking change, browser has no way of knowing.
<dandclark> sanketj_:For that case that's where inputType comes in, you should get insertReplacementText
<dandclark> sanketj_: The main thing is just we should make it clear that when one of these things are accepted, we should fire these events.
<dandclark> ...: Spec just says insert or replace text. Calls out type.
<dandclark> johanneswilm: Event order is in UI events, no?
<dandclark> sanketj_: Yes
<dandclark> sanketj_: Maybe somewhere in one of these specs, we should make it expicit that when suggestions are expected, these events get fired and composition events do not get fired
<dandclark> johanneswilm: Not against it, slightly afraid it's just due to lack of communication about the change
<dandclark> ...: But we can adjust it if it's not totally clear
<dandclark> ...: Should go into UI events
<dandclark> ...: I would be for having event order in same spec, but given we moved it out for everything else, keyboard typing etc, should be in UI events
<dandclark> ...: Maybe move the issue there, have them add it to spec where it seems reasonable
<dandclark> sanketj_: OK
<dandclark> johanneswilm: So all the event orders are in the same spec
<dandclark> sanketj_: Right. The other benefit is implementation-wise, once this is specced it can be forcing function to create common implementation. Rather than every time you invent one of these things you have to reconsider all this.
<dandclark> johanneswilm: Who will file it with UI events?
<dandclark> sanketj_: I'll follow up with Siye and he will take care of it

@johanneswilm johanneswilm removed the Agenda+ Queue this item for discussion at the next WG meeting label Jun 13, 2024
@garykac
Copy link
Member

garykac commented Jun 14, 2024

I don't think this belongs in UIEvents. I believe the whole point of a separate InputEvents spec is so that it can own all of the inputType-related information.

Since we're in the process of converting the UIEvents to be algorithmic (and eventually removing the "event ordering" tables), simply adding a note doesn't really fix the issue. Of course, since that conversion is ongoing (and the InputEvent section has not been addressed yet), this is a bit challenging at the moment.

However, I think this is the perfect time to start addressing that. What would such an algorithm look like? And where would it live?

As I said above, if a very editing-specific action like "replace text" doesn't belong in the InputEvents spec, then I wonder why we have a separate spec. (And to be clear, I think it's very beneficial to have it as a separate spec).

So that brings us back to: what would this algorithm look like? My first stab (without thinking about any subtleties) is something like:

In order to perform an editing action on the target, <a>send an input event sequence</a>
with the event target and the inputType. Valid inputTypes are described in the
following table:

Table of valid inputTypes:
    insertReplacementText
    ...

With the following algorithm added to UIEvents:

To <dfn>send an input event sequence</dfn>:
  : Input
  :: |target|, the {{EventTarget}} of the event
  :: |inputType|, the string identifying the type of string being performed.
      See [[InputEvents]].

  : Output
  :: None

  1. Let |beforeevent| = <a>create a cancelable InputEvent</a> with "beforeinput",
      |inputType|, |target|
  1. Let |result| = <a>dispatch</a> |beforeevent| at |target|
  1. If |result| is true, then do the following:
    1. Let |event| = <a>create an InputEvent</a> with "input", |inputType|, |target|
    1. <a>dispatch</a> |event| at |target|
  1. Otherwise:
    1. <handle canceled event>

Note that the other important part of this is having a description of how to handle the input event. I think this belongs in the InputEvents spec, although it may need refs to/from other specs.

To <dfn>handle the insertReplacementText input event</dfn>:
  : Input
  :: |target|, the {{EventTarget}} of the event (must be contenteditable?)

  : Output
  :: None

  1. <describe the steps needed to update the DOM>

Does this sound like a reasonable starting point?

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 26, 2024
When using autocorrect, we should use `insertReplacementText` according
to w3c/input-events#152. So I would like to
add eContentCommandReplaceText command for this.

Also, this command has an option that is source string text. When
processing text subsitution, parent process doesn't know whether
target replaced text is modified. So I add this option for check.

Differential Revision: https://phabricator.services.mozilla.com/D213511
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Jul 26, 2024
When using autocorrect, we should use `insertReplacementText` according
to w3c/input-events#152. So I would like to
add eContentCommandReplaceText command for this.

Also, this command has an option that is source string text. When
processing text subsitution, parent process doesn't know whether
target replaced text is modified. So I add this option for check.

Differential Revision: https://phabricator.services.mozilla.com/D213511
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Jul 27, 2024
When using autocorrect, we should use `insertReplacementText` according
to w3c/input-events#152. So I would like to
add eContentCommandReplaceText command for this.

Also, this command has an option that is source string text. When
processing text subsitution, parent process doesn't know whether
target replaced text is modified. So I add this option for check.

Differential Revision: https://phabricator.services.mozilla.com/D213511
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants