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

beforeinput shouldn't be fired before compositionstart for backward compatibility #86

Open
masayuki-nakano opened this issue Nov 10, 2018 · 8 comments

Comments

@masayuki-nakano
Copy link

Currently, 6. Event order during IME composition defines the order of beforeinput and compositionstart when there is selected content which will be replaced with composition string as:

  1. beforeinput with deleteByComposition
  2. input with deleteByComposition
  3. compositionstart

This means that the DOM tree is modified before compositionstart. However, currently, on Firefox, Chrome and Safari, the DOM tree is changed immediately before input event which follows compositionupdate. So, current draft's definition completely breaks backward compatibility if compositionstart event listener checks <input>.value or <div contenteditable>.innerHTML.

Additionally, data attribute of compositionstart is selected text which will be replaced. So, there is no reason to dispatch preceding beforeinput for deletion since web apps can cancel compositionstart (although Firefox still don't support cancelable `compositionstart).

Finally, when inserting text with pressing printable keys, only one pair of beforeinput and input events are fired, i.e., there are no preceding beforeinput and input for deleteContent. So, it's also bad for consistency.

@masayuki-nakano
Copy link
Author

Oh, but I think that deleteByComposition value is still useful if initial composition string is empty string and it replaces selected content before comositionstart.

@johanneswilm
Copy link
Contributor

Hey, I am posting this here although it could also be answered other places. I think it makes more sense to answer it in one go. I will then link here from the other places.

The point of the Input Events spec is that an editor can be built entirely by only listening to beforeinput events. So one shouldn't need to listen to composition events at all. Most of these beforeinput events are cancelable in Level 2, which is important because it means that the JavaScript can decide how to manipulate the DOM based on the user interaction.
Compositions are an exception because browsers are not allowed to interfere with the DOM while the composition takes place. So we discussed this for years and years on many meetings and long debates on email, and came up with a way of dealing with it: we simply trigger a cancelable deleteByComposition beforeinput event before the composition starts and a cancelable insertFromComposition beforeinput event at the end of it. This way the web app can still not interfere with the DOM during composition, but it can clean up the DOM before and after the composition takes place.

Level 1, on the other hand, was just a quick fix some people who had not been part of the discussions came up with to get around a problem Android IME has where the keyboard can trigger all kinds of events that the browser has no control over. In an attempt to make regular keyboard input just as bad as IME input, they also removed the cancelability of other text input events. Altogether level 1 is not really usable for much of anything except for checking format changes (add bold/italic - but this doesn't really apply to Firefox because you don't have those keyboard shortcuts), and possibly in case of some editors that are not constantly tracking the selection because it helps them determine where the caret was before a specific action was executed.

Implementing level 1 makes sense as long as it is a first step toward level 2. But by itself it's not useful. You can tell by these really big bugs being in there and noone having discovered them almost two years after it was shipped.

You mention that level 2 has some "unstable" elements and point to the order of events around composition [1]. Yet this is an issue that applies to level 1 and so in case there is no clarity around that, it is level 1 that is "unstable".

[1] #49 (comment)

@masayuki-nakano
Copy link
Author

Compositions are an exception because browsers are not allowed to interfere with the DOM while the composition takes place.

FYI: If browser is TSF-aware application like Firefox (and perhaps Edge), IME stops working entirely until browser process is restarted if web browser returns unexpected content to IME during composition when IME confirms whether inserting composition string works as expected or not. Therefore, web browsers, i.e., native applications, cannot behavior so tricky. (Gecko works hard to put off notifying IME of content changes as far as possible.)

we simply trigger a cancelable deleteByComposition beforeinput event before the composition starts and a cancelable insertFromComposition beforeinput event at the end of it. This way the web app can still not interfere with the DOM during composition, but it can clean up the DOM before and after the composition takes place.

I don't think that this can be a reason of current definition. Dispatching beforeinput with deleteByComposition means that IME has already sent native composition start event to browsers (i.e., native application). Therefore, composition has already started in IME. So, this is simply an issue of event order in the spec.

Implementing level 1 makes sense as long as it is a first step toward level 2.

Well, I don't say Mozilla won't implement Level 2, but I think that we need more time to implement it (both required changed of our design and some odd points in the spec).

You can tell by these really big bugs being in there and noone having discovered them almost two years after it was shipped.

Wait, no browser dispatches beforeinput and input before compositionstart. So, this spec issue hasn't been exposed to the web.

@johanneswilm
Copy link
Contributor

FYI: If browser is TSF-aware application like Firefox (and perhaps Edge), IME stops working entirely until browser process is restarted if web browser returns unexpected content to IME during composition when IME confirms whether inserting composition string works as expected or not. Therefore, web browsers, i.e., native applications, cannot behavior so tricky. (Gecko works hard to put off notifying IME of content changes as far as possible.)

Right, so it's really important we isolate the composition process and do changes to the DOM before and after.

I don't think that this can be a reason of current definition. Dispatching beforeinput with deleteByComposition means that IME has already sent native composition start event to browsers (i.e., native application). Therefore, composition has already started in IME. So, this is simply an issue of event order in the spec.

No, this is to happen before the composition starts (beforeinput with deleteByComposition is step 1, compositionstart is step 2). This is precisely done to isolate the composition. It is to explain this that we have added the explanation of events during IME is in the spec [1]. For level 1 both of these events were removed (as far as I can tell because the developers in question didn't take the time to participant in meetings and try to understand why they are essential to make the beforeinput event worthwhile). This is why that entire event sequence is missing from level 1. The order of beforeinput and composition events though is regulated by ui-events.

And yes, I can guarantee that is the reason for the current definition in level 2. This was largely thought up by @choniong (at that time Blink, now no longer working on editing), and I made sure it was working for JavaScript editors.

Wait, no browser dispatches beforeinput and input before compositionstart. So, this spec issue hasn't been exposed to the web.

Just the fact that Chrome hasn't shipped level 2 means that it has added little value to use the event in comparison with the old ways of doing it (waiting for DOM changes, then diffing the DOM). It will be different once more than just one browser ships level 2.

[1] https://w3c.github.io/input-events/#event-order-during-ime-composition

@johanneswilm
Copy link
Contributor

Wait, no browser dispatches beforeinput and input before compositionstart. So, this spec issue hasn't been exposed to the web.

I don't have an iphone, so I cannot test webkit on mobile, and I think that is the only place where one recomposes a string.

I can check Safari on MacOS, and there I can see that the event order at the end of the composition is slightly wrong (compositionend and insertFromComposition are backward). To web apps the order will only matter if canceling the beforeinput insertFromComposition will mean that the IME crashes and that one needs to restart the browser before the next composition.

@masayuki-nakano
Copy link
Author

I don't think that this can be a reason of current definition. Dispatching beforeinput with deleteByComposition means that IME has already sent native composition start event to browsers (i.e., native application). Therefore, composition has already started in IME. So, this is simply an issue of event order in the spec.

No, this is to happen before the composition starts (beforeinput with deleteByComposition is step 1, compositionstart is step 2).

Why you said "No"? I explained the behavior of native IME. Rarely, they may delete existing text with some complicated API set like TSF, but typically, they don't touch selected (non-composing) text before sending composition string to applications.

This is precisely done to isolate the composition. It is to explain this that we have added the explanation of events during IME is in the spec [1].

So, what I'm trying to say is, it may be difficult to implement such behavior with TSF. Some IME may stop working until restarting browser process since they meet unexpected situation.

Note that event order of composition/input events hasn't been declared so strictly. It makes sense for browser vendors because browsers just expose native IME behavior as DOM events, i.e., browsers can handle any tricky IME on the web. Of course, this could be nightmare of web app developers. So, all browsers should work similarly with same IME on same platform. However, IME may behave differently on each browser since IME can check focused application name in most platforms and they actually do so for avoiding some unexpected behavior of some native applications.

Wait, no browser dispatches beforeinput and input before compositionstart. So, this spec issue hasn't been exposed to the web.

Just the fact that Chrome hasn't shipped level 2 means that it has added little value to use the event in comparison with the old ways of doing it (waiting for DOM changes, then diffing the DOM). It will be different once more than just one browser ships level 2.

Safari has partially shipped Input Events Level 2, but they don't dispatch any new input events before compositionstart. The first input after compositionstart is the first event after the selected text is replaced with composition string. So, dispatching it can break existing web apps especially they do something without checking inputType value.

@johanneswilm
Copy link
Contributor

Why you said "No"? I explained the behavior of native IME.

I am slightly confused. Are you talking about how native IMEs work in other, native apps? Or are we talking about how the native IME communicates with the browser? The browser needs to be the ultimate authority in controlling the DOM and in deciding when events are triggered. I realize that historically there have been a ton of third party IMEs out there that have a lot of power and do a lot of uncontrolled actions. Without cracking down on that, we are never going to be in a situation where writing text can be done in predictable and error-free way. It seems Android has started doing that and I suspect Windows and other OSs as well.

Rarely, they may delete existing text with some complicated API set like TSF, but typically, they don't touch selected (non-composing) text before sending composition string to applications.

I realize that when you compose a new word, this is not really a problem as nothing has to be deleted. The problem starts when you recompose an existing bit of text. For example, on Android, if you touch a word, it starts out by loading that word into the IME. The browser needs to be the entity that needs to make sure that these events happen as stated in the spec and if that breaks the IME, then the IME devs have to fix their programs.

However, IME may behave differently on each browser since IME can check focused application name in most platforms and they actually do so for avoiding some unexpected behavior of some native applications.

So the IMEs are trying to work around the bugs in the browser's (neglected) contenteditable implementations? I can see how it theoretically can mess with such IMEs when the browsers are fixed, but that doesn't mean it's a bad idea to fix the browsers. On the contrary: If IMEs act differently based on which browser they are in, it shows that the devs behind them are actually interested in browsers and have spent some time to add temporary fixes in their software. Now that the browsers are fixed, there is a good chance that they are interested in fixing their code as well.

Safari has partially shipped Input Events Level 2 [...]

Safari has, as far as I can tell, shipped Level 2 completely. There are minor bugs in their implementation, some of which have been fixed and some of which have not been discovered yet.

So, dispatching it can break existing web apps especially they do something without checking inputType value.

Apps that rely on contenteditable to do something specific, or even worse, execCommand, are bound to break with just about every browser release. So that is not really an argument for not changing things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants