w3c / input-events

Input Events
https://w3c.github.io/input-events/
Other
23 stars 16 forks source link

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

Open siliu1 opened 1 month ago

siliu1 commented 1 month ago

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: https://github.com/whatwg/html/issues/10337

johanneswilm commented 1 month ago

@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?

siliu1 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

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/w3c/input-events/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
garykac commented 2 weeks ago

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?