w3c / input-events

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

Eliminate insertFromComposition, deleteByComposition, and deleteCompositionText #122

Closed BoCupp-Microsoft closed 2 years ago

BoCupp-Microsoft commented 3 years ago

This PR eliminates three inputTypes from beforeinput/input events: insertFromComposition, deleteByComposition, and deleteCompositionText.

It also clarifies the order of the insertCompositionText beforeinput and input events with respect to composition events. The proposed behavior is that beforeinput and input events for insertCompositionText occur only after compositionupdate events.

Additionally, a note stating that multiple compositionend events may occur for one compositionstart event was removed.

The result is that the composition sequence is:


Preview | Diff


Preview | Diff

johanneswilm commented 3 years ago

Hey, I don't think it would be a good idea to entirely remove them, because:

I do understand that Chromium has some issues with implementing these features. The Chromium implementation is not able to solve the issues that arise from not implementing this part in any other way. However, the beforeinput/input event is still a plus for other uses.

Therefore, I would propose to make these events optional instead of removing them entirely.

BoCupp-Microsoft commented 2 years ago

Hi @johanneswilm,

Sorry for the slow reply.

The challenge as I see it isn’t in implementing the events, but in supporting a relocatable selection when composition starts.

The goal of these events (per our previous conversations) is to enable an author to avoid having composition modify the portion of their DOM being used as the view for some editor-specific document model. The author is meant to accomplish that goal by relocating selection and focus to some other part of the DOM where the author would like composition to occur, and then, as composition progresses, the author can incorporate the result back into their document model so they can render it themselves as part of their view – presumably back to the same place where selection was when composition first started.

Let me know if you think I've stated the goal correctly and understood the way in which you expect the author to use these events.

My mental model of how to enable this scenario looks like this:

Step 1: During the compositionstart event, the author should move selection to a new editable element (call it the compositionElement) and remember where the selection was before the move (represent this location as a Range called originalCompositionRange).

Step 2: For each compositionupdate event, the author should process the data field into their editor-specific document model and render it back into originalCompositionRange. The author can repeatedly do this for every compositionupdate event, each time replacing the contents of the originalCompositionRange with new text. Note that since composition is taking place in the compositionElement and not inside the originalCompositionRange, the author is free to produce whatever markup they want inside the originalCompositionRange, e.g. the author might surround every character with a span.

Step 3: During the compositionend event, the author should move selection back to the end of originalCompositionRange and perform other necessary cleanup (like hiding the compositionElement).

Note one challenge with step 1 is that it isn’t clear to me how much additional context the author should copy from the original editable element into the compositionElement. On MacOS, the Japanese IME has a reconversion feature that expands the active composition range to the preceding phrase when the user presses left arrow. In order for that feature to work, the phrase to be included in the expansion must be in the compositionElement. Maybe the answer is to advise the author to copy a full paragraph of text?

Another challenge with step 1 is that the selection should indicate the text to be reconverted. This happens naturally when the text to reconvert is selected by the user. Reconversion can also be triggered, however, when the selection is collapsed. In this case the IME determines the range to reconvert. Firefox appears to handle this case by setting the selection equal to the reconversion range prior to dispatching the compositionstart event.

Some sample code showing the approach can be found here. It almost works in Firefox on Windows - the first character typed in the composition is dropped as a result of moving the selection to the compositionElement, but it’s otherwise functional. The first character issue I believe is unrelated to whether these events are kept or removed.

It also almost works in Safari, but to facilitate reconversion and workaround selection not being equal to the reconversion range, I produced another variant here which derives the reconversion range from the first insertCompositionText beforeinput event. This works for Safari on MacOS for all cases except when the user wants to expand the reconversion range using the left arrow. For that scenario to work I further modified the code to this final variant which also handles insertFromComposition and deleteCompositionText by performing the same basic steps: replace the contents of originalCompositionRange with the data from the event.

Besides Firefox on Windows and Safari on MacOS, other browsers have more significant problems with all variants.

In the final variant I mention above for Safari there are also side effects on the state of the IME where it is taken out of phrase mode when hitting the left arrow.

Based on my observations from these experiments, my intuition is that there’s a long tail of issues to sort through both for the spec and implementations as we discover the sensitivities of various IMEs for different languages and different platforms. When I consider that the EditContext accomplishes the same goal but doesn’t require a complex protocol to deal with focus changes during composition start and can also work for other scenarios like an editor that renders to a canvas – I'm reluctant to invest more in making a relocatable selection work.

Could we instead declare that relocating selection during compositionstart is unsupported and remove these superflous events?

johanneswilm commented 2 years ago

Hey @BoCupp-Microsoft, thanks for experimenting. These events were created after discussions involving tens of subject experts over an extended period of time. Moving the caret somewhere else is just one of multiple possible uses. Another is to leave the composition in its original place but to use the events at the end of the composition to do some editor-related cleanup of the dom. For example, an editor may want to add spans to mark who wrote a particular section of the document or to mark at which time it was written. Spans could also be used for a word-style tracked changes feature. It is possible that EditContext will cover these cases. Have all three browser engines committed to fully implement EditContext within a reasonable timeframe? If not, I'd think it would be premature to remove the only currently working implementation of a spec that actually addresses the need to do this. Why can we not do as we planned initially - mark these events as optional so that we can merge level 1 and 2 because Chromium has technical issues implementing this, but not remove them entirely so that webkit doesn't lose it's status as being compliant? If you do think we should still go ahead and remove them entirely, can we have a meeting involving the subject experts that came up with the current solution first?

BoCupp-Microsoft commented 2 years ago

Another is to leave the composition in its original place but to use the events at the end of the composition to do some editor-related cleanup of the dom. For example, an editor may want to add spans to mark who wrote a particular section of the document or to mark at which time it was written. Spans could also be used for a word-style tracked changes feature.

For this scenario what prevents authors from using compositionend?

Why can we not do as we planned initially - mark these events as optional so that we can merge level 1 and 2 because Chromium has technical issues implementing this, but not remove them entirely so that webkit doesn't lose its status as being compliant?

That's fine. I'll stop objecting. Do you want to prepare that pull request and I'll abandon this one?

johanneswilm commented 2 years ago

For this scenario what prevents authors from using compositionend?

In practice, compositionend, at times in combination with a timeout, is what is used in a lot fo editors today as it is one fo few events that is universally available. However, the event does not give enough information to make it clear where in the dom which changes were done. So with the event, one has to trigger something else - for example a dom diffing mechanism. That's what we attempted to get away from by giving the required information to the web dev so cleanup could be doing with diffing.

That's fine. I'll stop objecting. Do you want to prepare that pull request and I'll abandon this one?

OK, I can do that.

BoCupp-Microsoft commented 2 years ago

@masayuki-nakano, @johanneswilm, @whsieh

In this PR I was proposing we eliminate three input events that aren't implemented by Chromium browsers or Firefox. I think the events are unnecessary, but the events are currently implemented in Safari. Johannes asked if we could mark the events optional instead of eliminating them. I said above that I wouldn't object.

@whsieh and @masayuki-nakano could you let @johanneswilm and I know if you're happy with that outcome? If so, I think we can cancel the meeting we were planning for this Friday to discuss how to move forward with this spec. If you think we should discuss other options we can continue discussion here and in our ad-hoc meeting of the Web Editing WG this Friday.

Thanks for your thoughts! Bo

BoCupp-Microsoft commented 2 years ago

Tagging related issue: https://github.com/w3c/input-events/issues/115

masayuki-nakano commented 2 years ago

I think that deleteByComposition and deleteCompositionText should not be fired (especially for input) from the draft (https://github.com/w3c/input-events/issues/86) because in my experience, changing dispatching event timing and/or count is risky change for existing web apps.

On the other hand, about insertFromComposition, I sometimes get feedback that web apps cannot cancel committing composition makes them require MutationObserver etc. But I think that it is not necessary if insertCompositionText can be cancelable only when committing the composition (https://github.com/w3c/input-events/issues/115) because I don't think that inputType is referred strictly from web apps except some special types like insertFromPaste.

So basically, I'd be happy if they'd be dropped from the draft because of the backward compatibility.

johanneswilm commented 2 years ago

So basically, I'd be happy if they'd be dropped from the draft because of the backward compatibility.

The current proposal is that they are not dropped but made optional. Are you ok with that @masayuki-nakano?

masayuki-nakano commented 2 years ago

So basically, I'd be happy if they'd be dropped from the draft because of the backward compatibility.

The current proposal is that they are not dropped but made optional. Are you ok with that @masayuki-nakano?

Well, I don't like the unclear state, but it's fine to me (If Chromium would support them, I'd be not happy, but probably we'll follow it). On the other hand, I don't know whether such unclear state makes web developers confused or not. I guess they'd be confused... Probably, the spec should explain the reason in a note.

johanneswilm commented 2 years ago

Well, I don't like the unclear state, but it's fine to me (If Chromium would support them, I'd be not happy, but probably we'll follow it). On the other hand, I don't know whether such unclear state makes web developers confused or not. I guess they'd be confused... Probably, the spec should explain the reason in a note.

Ok, will add a note.

BoCupp-Microsoft commented 2 years ago

On the other hand, about insertFromComposition, I sometimes get feedback that web apps cannot cancel committing composition makes them require MutationObserver etc. But I think that it is not necessary if insertCompositionText can be cancelable only when committing the composition (#115) because I don't think that inputType is referred strictly from web apps except some special types like insertFromPaste.

I'd like to better understand this comment. Why would cancellation only during commit be useful? The DOM is modified after every compositionupdate / insertCompositionText. Why put special requirements around the last one?

masayuki-nakano commented 2 years ago

On the other hand, about insertFromComposition, I sometimes get feedback that web apps cannot cancel committing composition makes them require MutationObserver etc. But I think that it is not necessary if insertCompositionText can be cancelable only when committing the composition (#115) because I don't think that inputType is referred strictly from web apps except some special types like insertFromPaste.

I'd like to better understand this comment. Why would cancellation only during commit be useful? The DOM is modified after every compositionupdate / insertCompositionText. Why put special requirements around the last one?

The merit of beforeinput is that it's possible to cancel user input before DOM tree update. Therefore they may want to cancel the final input from IME composition, e.g., perhaps for filtering characters. Currently other insertFrom* and insertText are canceleable, but not for any IME compositions. Therefore, web app developers need special handling only for IME. Therefore, this is unhappy case for both web developers and IME users.

Note that canceling "composing characters" may make serious a11y issue. E.g., cannot see characters which are required to convert, and composing character should not be filtered because there are some cases that composing characters are not acceptable but final string is acceptable, e.g., converting from CJK names to their emails. Therefore, beforeinput for composing state shouldn't be cancelable.

BoCupp-Microsoft commented 2 years ago

The merit of beforeinput is that it's possible to cancel user input before DOM tree update.

Agree. What I don't agree with is that cancellation of just the final committed text is important. If the author can't stop composition from modifying their DOM for the first N compositionupdate events, then being able to cancel N+1 doesn't seem to matter. The author still needs to deal with cleaning up their DOM and can process the data of the last compositionupdate by either pulling the final composition string from the DOM or from the event's data property. I can't see how being able to cancel just the last event helps.

masayuki-nakano commented 2 years ago

What I don't agree with is that cancellation of just the final committed text is important. If the author can't stop composition from modifying their DOM for the first N compositionupdate events, then being able to cancel N+1 doesn't seem to matter.

Indeed, if web apps so sensitive for DOM mutations, you're right. But I guess some apps may not be so sensitive, but may want to inserting the string by themselves. I assume that canceling the final beforeinput event for a composition deletes composing text by web browser, not keeps it as-is.

css-meeting-bot commented 2 years ago

The Web Editing Working Group just discussed Eliminate insertFromComposition, deleteByComposition, and deleteCompositionText.

The full IRC log of that discussion <Travis> topic: Eliminate insertFromComposition, deleteByComposition, and deleteCompositionText
<Travis> github: https://github.com/w3c/input-events/pull/122
<tilgovi> johanneswilm: There were years of discussions for input events. In the end, it was implemented almost at the same time in WebKit and Chromium. WebKit implemented it the way we concluded, but Blink figured out there were parts that were difficult to implement. They couldn't implement it all.
<tilgovi> ... What we decided was to divide the specification into two levels so we could get them to implement part of it. Level 1 would contain the things that the Chromium team could implement and Level 2 would contain the proposal of what we wanted, and what WebKit implemented.
<tilgovi> ... Someone figured out that actually Chromium start implementing things from Level 2 anyway. It turned out that a lot of it was possible. Now Chromium is shipping most of Level 2 anyway, with very few exceptions related to IME.
<tilgovi> ... Now that they're almost the same, maybe we can scrap Level 1 and just have the Level 2.
<tilgovi> ... Bo did some investigation about what's possible and what's not, and suggested let's remove some input events from Safari, and we came to an agreement to mark them as optional in the spec. We merged Level 1 and Level 2 and marked three input events as optional.
<Travis> q?
<tilgovi> ... Then, Firefox has implemented it in the same way as Chromium. The representative from Firefox said we need to have a note while these are marked as optional, explaining some informative background. That's the agreement we currently have that everyone seems okay with to merge Level 1 and Level 2.
<BoCupp> q+
<tilgovi> ... This way, all browsers will be compliant.
<Travis> ack BoCupp
<tilgovi> BoCupp: There's a lot of differences between browsers for input events. In this one aspect, can we agree on the set of events that should exist? I think we're there with the exception of these three events. Firefox and Chrome do not dispatch these beforeinput events and Safari currently does. The PR is just about can we say that these beforeinput events MAY be dispatched.
<tilgovi> johanneswilm: Currently, they are required. What we need consensus on is to mark them optional. You originally proposed to eliminate them?
<tilgovi> BoCupp: That's right. But I am okay with the "MAY" keyword in there.
<tilgovi> annevk: Why are the events problematic?
<tilgovi> BoCupp: The sequence of events removes content from the DOM and then re-inserts it. On the Chromium side that's an unnecessary mutation. It's extra churn in the DOM. To have a special dance around the beginning and end of composition more than to mark that with events does not seem to have particular value.
<BoCupp> q+
<snianu> q?
<tilgovi> My laptop just shut off without apparent reason, so you're without scribe until I boot back up.
<Travis> ack BoCupp
<Travis> Note: about 5 minutes left in the meeting.
<Travis> tilgovi: thanks!
<Travis> scribeNick: Travis
<Travis> whsieh: I think there is still value in seeing text being committed at the end of the composition.
<Travis> .. note you can move around the "phrase" in some IMEs?
<Travis> BoCupp: Yes, but the range is the entire string--not sub-sets.
<Travis> .. there's an initial composition range, and as you type things are changing... but the whole string is being replaced in the range.
<Travis> johanneswilm: What happens when you hit a buffer limit?
<Travis> BoCupp: What if you need to change the active composition range? (Not just for max limits).
<Travis> .. the composition ends (and re-starts).
<Travis> .. so any one of these ranges are sufficient.
<Travis> .. in Firefox, the selection is updated immediately, so you can derive it from the selection (don't need the composition events!)
<BoCupp> q?
<Travis> .. in closing: I'm OK with saying the events are optional.
<Travis> .. masayuki (based on his posts), thinks its ok if these are optional.
<Travis> johanneswilm: I think he also wants there to be a note explaining why.
<Travis> annevk: Yes, masayuki does represent Firefox on this matter.
<Travis> .. Want to ask whsieh (or someone else at Apple) to take a look before a decision is made.
<Travis> whsieh: bo, when you want to remove the evnets, would you replace that with a regular insertText?
<Travis> BoCupp: There's two beforeinput events that come at the end... one at the front.
<Travis> .. I would not dispatch the one before composition starts.
<Travis> .. each sequence of compositionupdate would be paired with an insertText...
<Travis> .. then the compositionend would be the end.
<Travis> .. also proposing removing the event that fires (only during reconversion): deleteByComposition
<Travis> BoCupp: would love to have whsieh take a look and see if it could be removed...
<Travis> whsieh: we've shipped this for many years--could have compat risk to remove (in Apple ecosystem).
<Travis> BoCupp: likewise could be compat issues to add them :-) (in Firefox/Chromium)
<Travis> annevk: See value in taking another look.
<Travis> whsieh: possible to switch behavior, but we try to avoid it if possible.
<Travis> johanneswilm: This was meant, in the end, to help JS editors.
<Travis> snianu: EditContext has a lot more flexibility for this (it doesn't update the DOM)
<Travis> BoCupp: would like to direct folks attention to an update to EditContext that is out for review.
<Travis> https://w3c.github.io/editing/docs/EditContext/index.html
johanneswilm commented 2 years ago

For reference, in this issue @chong-z explained why we added these events in the first place: https://github.com/w3c/input-events/issues/34

johanneswilm commented 2 years ago

@BoCupp-Microsoft I don't see us having decided on any action at the meeting. Is that also what you recall from the meeting? What do you propose we do at this stage given that the positions are the ones they are? As far as I can tell, currently it says that the events are required. AFAICT, there does not seem to be consensus for removing them, but there seems to be potential consensus for marking them optional. Is that also your understanding? We cancelled the meeting last week, but if there are still things to clarify, maybe we should need another special meeting? I would think it would mainly make sense if there are new people or new views that potentially could take part in such a debate. Just having the same people mention the same points again seems less fruitful. In that case I'd say we should probably resolve to just keeping things as they are with two different levels of the spec until such time that something changes the circumstances.

css-meeting-bot commented 2 years ago

The Web Editing Working Group just discussed pull requests on input-events (they are basically the same thing).

The full IRC log of that discussion <Travis> topic: pull requests on input-events (they are basically the same thing)
<Travis> github: https://github.com/w3c/input-events/pull/122
<Travis> johanneswilm: want to know if we have more information here...
<Travis> .. looking to see if we can mark these as not required in the spec?
<Travis> whsieh: to be clear, both alternatives are OK: remove them, or mark them as deprecated in the spec.
<Travis> .. may be a precident for similar things marked deprecated but supported by webkit? extendSelection?
<Travis> johanneswilm: what is desired?
<Travis> BoCupp: If whsieh is fine with removing it (from both spec/webkit), that would be my preference.
<Travis> action: land this pull request (that removes these events from the spec); expectation that they may get removed from webkit in the future.
whsieh commented 2 years ago

I think we (WebKit) would be okay with removing support for these three input event types, to align with Gecko and Blink.

johanneswilm commented 2 years ago

think we (WebKit) would be

@whsieh OK, can we get confirmation on that that it is definitely OK so that we can go ahead and merge this.

DavidMulder0 commented 2 years ago

As someone who develops a browser based structured document editor I am confused and disheartened seeing that deleteByComposition is going to get removed. Right now our editor is read-only in Android Chromium based browsers, because the deleteByComposition isn't supported. As far as we are aware there is literally no way to support the Chrome/Edge for Android recomposition flow at the moment, as there is no way to know what part of the DOM is being changed. We have mocked up certain prototypes that would diff the DOM before and after composition, but given the nature of our editor this has proven unfeasible to do in a generic way. The - as far as we were aware - upcoming deleteByComposition allowed us to promise our customers that Android support would be forthcoming eventually, but now it seems this change would move support to a 'maybe one day Chromium will support EditContext'.

We went through the same testing @BoCupp-Microsoft described where we tried to change the selection in response to the compositionstart event, but this (unsurprisingly) turned out to be mired with issues, some of which BoCupp described[1]. We settled on a solution where we allow the composition events to uncontrollably pollute our DOM, after which we rerender the entire section of the document when the composition finishes using the InputEvent.data to know what to insert. Without deleteByComposition this fails in the case of recomposition though, so basically from our point of view deleteByComposition is the crucial absolutely necessary glue to make Input Events have any meaning. This might sound harsh, but without it the entire spec seems to do more harm than good[2].

In conclusion: Maybe we're missing something (we well might be, mobile device support is low on our list of priorities, so most of the investigations have been done by a single person: me), but we really feel like these 3 events are crucial additions to the Input Events API. Even if EditContext will provide a solution that we would like to use one day, that shouldn't be a reason to handicap this spec indefinitely.

[1] We also investigated an approach where we would consistently keep the selection 'outside' the actually editable document, but this turned out to result in too much unpredictable behavior with different IME solutions (although we're aware that Google Docs was able to get this to work well). [2] Reason I am saying this is because the spec creates the illusion that a good editor can be created in the browser relatively easily. Next you spend years (literally) developing a product just to find out that you basically can't fully support IME input. Luckily IME support can be decently hacked at level 1 as long as no recomposition happens... but the fact that theoretically any IME might break your editor is... scary.

masayuki-nakano commented 2 years ago

As far as we are aware there is literally no way to support the Chrome/Edge for Android recomposition flow at the moment, as there is no way to know what part of the DOM is being changed.

Cannot retrieve the range to be recomposed with Selection at compositionstart and/or InputEvent.getTargetRanges at beforeinput whose inputType is insertCompositionText? If not, I think that it's a bug of Blink.

johanneswilm commented 2 years ago

@BoCupp-Microsoft Do you have an answer to the above concern?

DavidMulder0 commented 2 years ago

@masayuki-nakano Right now out of the subject matter a bit, but the main problem I am aware of is that IME's really do not like it when anybody 'messes' with their composition area. Even if Chrome would correctly provide getTargetRanges on insertCompositionText this wouldn't allow updating the underlying data model, as rerendering the DOM will cause many (if not all) IME's to end the composition session (as far as I understand that's the reason why preventDefault isn't supported on composition input events). Theoretically however one could maybe track and combine the getTargetRanges on the client to only do the update once the composition ends, basically replicating the insertFromComposition manually.

Still though, this results in a situation where I think a lot of implementations are going to be incorrect, as insertCompositionText will need a very uniquely different implementation from all other input events. Plus it requires this 'odd' interplay between the composition events to determine which insertCompositionText was last, to correctly handle undo/redo actions (as a compositionupdate should probably not trigger its own undo action).

To be fair, it's all doable and when I wrote the last comment I somehow hadn't noticed that getTargetRanges actually should be set for composition related events as well, so thanks for pointing that out.

johanneswilm commented 2 years ago

@DavidMulder0 Your last paragraph makes it sound like you see less of a problem with this change now than previously. Is that correctly understood, or do you still view removing these three types permanently from the spec as problematic?

BoCupp-Microsoft commented 2 years ago

BoCupp-Microsoft Do you have an answer to the above concern?

The first beforeinput insertCompositionText should have the range to be reconverted marked before the text is updated. That makes reconversion the same as every other case from the author's perspective: the range of insertCompositionText is indicating what will be replaced.

I would also be OK with doing as @masayuki-nakano suggests and making the selection match that range before dispatching compositionstart. We don't currently do that in Chromium, and I'm not sure its necessary given we'll report the same range in the following insertCompositionText event, but I'd be willing to make a change for interoperability.

BoCupp-Microsoft commented 2 years ago

@DavidMulder0 could you confirm if insertCompositionText would meet your needs as I've described it being used above?

css-meeting-bot commented 2 years ago

The Web Editing Working Group just discussed Input events (PR 122).

The full IRC log of that discussion <Travis> Topic: Input events (PR 122)
<Travis> github: https://github.com/w3c/input-events/pull/122
<Travis> johanneswilm: Was going to merge this PR, but wanted to get additional confirmation from the group...
<Travis> .. wanted to get implementer confirmation that we aren't removing something important.
<Travis> BoCupp: yes, I will respond
<Travis> johanneswilm: seems the commenter is actually using these events in their editor, and can't see an alternative.
<Travis> BoCupp: assuming, this meant how to get the range for what is going to be converted. The IME decides what those extents are. The first CompositionUpdate is no different from any other, and the range is the same each time...
<Travis> .. also masayuki suggested that you could use selection... but this is not implemented in chromium.
<Travis> .. by the time the compositionstart fired, firefox will expand the range to encompass the part that will be reconverted.
<Travis> .. I think this is unique to firefox. I'm OK with this being sanctioned as good behavior--however, I think it might be unnecessary because insertCompositionText gives you the same thing.
<Travis> .. I've been pushing that insertCompositionText be the same (dependable) each time. Hoping that makes it easier vs using the other events.
<Travis> .. I will post on the issue regarding this.
<Travis> .. Might spin off a separate issue.
<Travis> johanneswilm: user claims that there may be a way around it. Bo, please continue discussing to get clarity. If your proposed solution...
<Travis> .. to determine the range to be reconverted, one can use getTargetRanges of the first insertCompositionText event...
<Travis> .. be filed as a new issue.
masayuki-nakano commented 2 years ago

@DavidMulder0

@masayuki-nakano Right now out of the subject matter a bit, but the main problem I am aware of is that IME's really do not like it when anybody 'messes' with their composition area.

Yes. If native (i.e., OS's) IME API in all platforms works smarter, browsers can do better. However, for example, IME on Windodws handling with TSF may stop working until restarting browser if the composing string is unexpected.

as rerendering the DOM will cause many (if not all) IME's to end the composition session (as far as I understand that's the reason why preventDefault isn't supported on composition input events).

What should happen when IME gets unexpected result depends on IME in any platforms. I.e., out of scope of browsers. Therefore, preventDefault during composition is hard to implement.

Plus it requires this 'odd' interplay between the composition events to determine which insertCompositionText was last, to correctly handle undo/redo actions (as a compositionupdate should probably not trigger its own undo action).

For solving such non-smart API, I suggested InputEvent.isComposing, but there are some problems, see https://github.com/w3c/uievents/issues/202 for the detail.

Basically, while IME has composition string browser nor web app should not do anything which may surprise IME, e.g., sanitizing the composing string, moving to different position. When the composition gets committed, browser and/or web app should put the result into its undo stack. Then, IME features except undoing last commit feature should work.

Therefore, I think that the last insertCompositionText of a composition could be cancelable (https://github.com/w3c/input-events/issues/115).

@BoCupp-Microsoft

I would also be OK with doing as @masayuki-nakano suggests and making the selection match that range before dispatching compositionstart.

IME for TSF on Windows do same thing. First, setting selection to replace, then, insert (replace) text. Therefore I'm surprised at why Blink does not do this on Windows. And IIRC, reconversion on macOS is also setting selection first. So, doing it is reasonable behavior to me...

johanneswilm commented 2 years ago

@DavidMulder0 @BoCupp-Microsoft @masayuki-nakano Do you feel that we have reached a conclusion here and that this PR can be merged as is, or do we need to modify it?

css-meeting-bot commented 2 years ago

The Web Editing Working Group just discussed Input events.

The full IRC log of that discussion <Travis> Topic: Input events
<Travis> github: https://github.com/w3c/input-events/pull/122
<Travis> johanneswilm: Is there agreement on what to do next?
<Travis> .. please follow up BoCupp !
css-meeting-bot commented 2 years ago

The Web Editing Working Group just discussed Anything futher on insertFrom... deleteBy.. etc.?.

The full IRC log of that discussion <Travis> Topic: Anything futher on insertFrom... deleteBy.. etc.?
<Travis> github: https://github.com/w3c/input-events/pull/122
<Travis> BoCupp: Are we still exploring other options?
<Travis> scribe: Travis
<Travis> johanneswilm: Actually... we're on 122 w3c/input-events
<Travis> .. is there consensus on this?
<Travis> BoCupp: Last status... on 12/10 we were asking DavidMulder...
<Travis> johanneswilm: So, you and masayuki are in agreement?
<Travis> BoCupp: Yes, I think so.
<Travis> johanneswilm: BoCupp could you please confirm that this is ready to merge?
<Travis> BoCupp: looked like masayuki was answering david's question... I think earlier in the thread it looked like masayuki would be disappointed if chromium did add these...
<Travis> .. will double check, and post to the issue.
BoCupp-Microsoft commented 2 years ago

Apologies, I was thinking this was ready to merge because @masayuki-nakano and I were in agreement and I didn't see any objection from @DavidMulder0. When I read back through this thread though I see this comment from @masayuki-nakano that I didn't address:

I assume that canceling the final beforeinput event for a composition deletes composing text by web browser, not keeps it as-is.

With this PR (specifically with the removal of deleteCompositionText) there is no operation that deletes the composed text as part of committing it. If the author wants to remove the text from the DOM and then reinsert it themselves, I believe they are free to do so using the last reported range of the beforeinput event which precedes compositionend.

The code would look like this:

const ce = document.querySelector("[contenteditable]")      

let lastRange = null
ce.addEventListener("beforeinput", e => {
  if (e.inputType != "insertCompositionText") {
    return
  }

  const ranges = e.getTargetRanges()
  if (ranges.length) {
    lastRange = ranges[0]
  }
})

ce.addEventListener("compositionend", e => {
  if (lastRange) {
    // StaticRange doesn't have a deleteContents, so convert to live Range
    const r = new Range()
    r.setStart(lastRange.startContainer, lastRange.startOffset)
    r.setEnd(lastRange.endContainer, lastRange.endOffset)
    r.deleteContents()
  }
})

There's a demo here that logs events so you can clearly see what's happening. I tested it in FireFox with the in-box Japanese IME on Mac and Windows. @masayuki-nakano, let me know if you agree to having the author handle the scenario you raised as I have shown above. If so, then I don't think we need any additional behavior to remove the composed text on the final beforeinput preceding compositionend and we can merge this PR as is.

masayuki-nakano commented 2 years ago

@BoCupp-Microsoft Sorry for the delay to reply, I got lost because of too busy days due to a house moving. And thank you for pinging me, @johanneswilm.

With this PR (specifically with the removal of deleteCompositionText) there is no operation that deletes the composed text as part of committing it. If the author wants to remove the text from the DOM and then reinsert it themselves, I believe they are free to do so using the last reported range of the beforeinput event which precedes compositionend.

From point of view of editor app developers, I agree with you, however, according to some bug reports to Firefox, some (non-editor apps') web developers want to use beforeinput lighter. Your idea requires web apps need to implement undo transaction by themselves. However, such developers want to do that instead, composition should be cancelable if the final value is not acceptable, but undo transactions should keep working.

The code would look like this:

const ce = document.querySelector("[contenteditable]")      

let lastRange = null
ce.addEventListener("beforeinput", e => {
  if (e.inputType != "insertCompositionText") {
    return
  }

  const ranges = e.getTargetRanges()
  if (ranges.length) {
    lastRange = ranges[0]
  }
})

ce.addEventListener("compositionend", e => {
  if (lastRange) {
    // StaticRange doesn't have a deleteContents, so convert to live Range
    const r = new Range()
    r.setStart(lastRange.startContainer, lastRange.startOffset)
    r.setEnd(lastRange.endContainer, lastRange.endOffset)
    r.deleteContents()
  }
})

There's a demo here that logs events so you can clearly see what's happening. I tested it in FireFox with the in-box Japanese IME on Mac and Windows. @masayuki-nakano, let me know if you agree to having the author handle the scenario you raised as I have shown above. If so, then I don't think we need any additional behavior to remove the composed text on the final beforeinput preceding compositionend and we can merge this PR as is.

So as far as I've tested, the demo breaks undo/redo behavior at least on both Firefox and Chrome. Therefore, I think that final composition being cancelable is reasonable to me, and it's consistent with the other text insertion inputTypes.

On the other hand, this PR is a reasonable size, and including only one purpose, "removing the unnecessary inputType values". It seems that discussing and updating the specification in another PR may be better.

BoCupp-Microsoft commented 2 years ago

Thank you for the feedback @masayuki-nakano. I'll open a separate issue to discuss cancellation for the final composition per this comment and merge this PR:

On the other hand, this PR is a reasonable size, and including only one purpose, "removing the unnecessary inputType values". It seems that discussing and updating the specification in another PR may be better.

BoCupp-Microsoft commented 2 years ago

Created this issue to discuss whether the last beforeinput before compositionend should be cancellable per this comment.