[editing-explainer] beforeinput should be fired only when the DOM change is caused by direct input by keypress or composition (#43)

Copied from DOML3 Bugzilla https://www.w3.org/Bugs/Public/show_bug.cgi?id=23913


Masayuki Nakano     2013-11-25 10:03:05 UTC  
I strongly believe that beforeinput should be fired only when:

* the DOM change is caused by printable key press event (or Enter key etc.)
* the DOM change is caused by composition (i.e., IME)

The reason is, if beforeinput is fired at all DOM changes, the implementation becomes very complicated and web apps must not want the events which are caused by value change by their script.

Additionally, if we allow to dispatch beforeinput for all DOM changes, it may cause nested events. E.g., in beforeinput event handler, script changes the editor value again, beforeinput is fired again *before* finishes the previous beforeinput dispatching. This is dangerous.

Similarly, editor commands shouldn't cause beforeinput event since they can be kicked by execCommand() method.


Comment 1  Ojan Vafai     2013-11-25 23:57:06 UTC  
I think you can generalize to firing beforeinput only for user-actions, not script. I can think of other user actions that you'd want beforeinput to fire for than just key input (e.g. cut/paste from a menu). 

This is already how the input event works, so having beforeinput match that makes sense.


Comment 2  Masayuki Nakano     2013-11-26 00:13:58 UTC  
(In reply to Ojan Vafai from comment #1)
> I think you can generalize to firing beforeinput only for user-actions, not
> script. I can think of other user actions that you'd want beforeinput to
> fire for than just key input (e.g. cut/paste from a menu). 

I don't want to fire beforeinput at cut/paste. As I said, it can be performed from JS. Additionally, Gecko cannot distinguish if the editing action is performed from JS or user action. If it should be fired only when user executes it, we need for several years for implementing this feature.

> This is already how the input event works, so having beforeinput match that
> makes sense.

At least on Gecko, input event is fired when the editor value is changed always.

The important difference between beforeinput and input is, beforeinput is cancellable. I.e., it MUST be fired before modifying editor value. This means that beforeinput may be nested if beforeinput handler causes another beforeinput. (input event isn't cancellable, therefore, browser can wait safe time to dispatch input event.)


Comment 3  Ojan Vafai     2013-11-26 00:19:01 UTC  
(In reply to Masayuki Nakano from comment #2)
> (In reply to Ojan Vafai from comment #1)
> > I think you can generalize to firing beforeinput only for user-actions, not
> > script. I can think of other user actions that you'd want beforeinput to
> > fire for than just key input (e.g. cut/paste from a menu). 
> 
> I don't want to fire beforeinput at cut/paste.

Why?

> As I said, it can be
> performed from JS. Additionally, Gecko cannot distinguish if the editing
> action is performed from JS or user action. If it should be fired only when
> user executes it, we need for several years for implementing this feature.

That seems like a bug Gecko needs to fix. Doesn't make sense to define the standard around a Gecko bug.

> > This is already how the input event works, so having beforeinput match that
> > makes sense.
> 
> At least on Gecko, input event is fired when the editor value is changed
> always.

In this simple test case, that doesn't seem to be the case:
http://plexode.com/eval3/#s=aekVQXANJVQMbAx1VRllVQlNGQgFKRR5HUFAfHRCWmJqcHz1PHXKeoE1QSEhGU6UQch8DDQNLkZNwD24JCKJQCAoPUE91AR4BYAkKAVytAdkBw8UIs7W3yg9oAQzRCHUdQ7gIHK1era3cxsjiV0JNVkbQAcejdoDLwAA=

> The important difference between beforeinput and input is, beforeinput is
> cancellable. I.e., it MUST be fired before modifying editor value. This
> means that beforeinput may be nested if beforeinput handler causes another
> beforeinput. (input event isn't cancellable, therefore, browser can wait
> safe time to dispatch input event.)

I don't think it makes sense for beforeinput and input to fire for a different set of actions.


Comment 4  Masayuki Nakano     2013-11-26 00:37:52 UTC  
(In reply to Ojan Vafai from comment #3)
> (In reply to Masayuki Nakano from comment #2)
> > (In reply to Ojan Vafai from comment #1)
> > > I think you can generalize to firing beforeinput only for user-actions, not
> > > script. I can think of other user actions that you'd want beforeinput to
> > > fire for than just key input (e.g. cut/paste from a menu). 
> > 
> > I don't want to fire beforeinput at cut/paste.
> 
> Why?
> 
> > As I said, it can be
> > performed from JS. Additionally, Gecko cannot distinguish if the editing
> > action is performed from JS or user action. If it should be fired only when
> > user executes it, we need for several years for implementing this feature.
> 
> That seems like a bug Gecko needs to fix. Doesn't make sense to define the
> standard around a Gecko bug.

I don't think that it's very hard to implement on some browsers, the spec won't available at least for a long time. It doesn't make happy anybody.

> > > This is already how the input event works, so having beforeinput match that
> > > makes sense.
> > 
> > At least on Gecko, input event is fired when the editor value is changed
> > always.
> 
> In this simple test case, that doesn't seem to be the case:
> http://plexode.com/eval3/
> #s=aekVQXANJVQMbAx1VRllVQlNGQgFKRR5HUFAfHRCWmJqcHz1PHXKeoE1QSEhGU6UQch8DDQNLk
> ZNwD24JCKJQCAoPUE91AR4BYAkKAVytAdkBw8UIs7W3yg9oAQzRCHUdQ7gIHK1era3cxsjiV0JNVk
> bQAcejdoDLwAA=

Does it true for all cases? E.g., execCommand().

> > The important difference between beforeinput and input is, beforeinput is
> > cancellable. I.e., it MUST be fired before modifying editor value. This
> > means that beforeinput may be nested if beforeinput handler causes another
> > beforeinput. (input event isn't cancellable, therefore, browser can wait
> > safe time to dispatch input event.)
> 
> I don't think it makes sense for beforeinput and input to fire for a
> different set of actions.

beforeinput is used for cancelling user input. On the other hand, input is used for taking some reaction for user input. So, beforeinput only fired at caused by cancellable action does makes sense even if there are some cases input event is fired without beforeinput.


Comment 5  Ojan Vafai     2013-11-26 01:30:54 UTC  
> > > > This is already how the input event works, so having beforeinput match that
> > > > makes sense.
> > > 
> > > At least on Gecko, input event is fired when the editor value is changed
> > > always.
> > 
> > In this simple test case, that doesn't seem to be the case:
> > http://plexode.com/eval3/
> > #s=aekVQXANJVQMbAx1VRllVQlNGQgFKRR5HUFAfHRCWmJqcHz1PHXKeoE1QSEhGU6UQch8DDQNLk
> > ZNwD24JCKJQCAoPUE91AR4BYAkKAVytAdkBw8UIs7W3yg9oAQzRCHUdQ7gIHK1era3cxsjiV0JNVk
> > bQAcejdoDLwAA=
> 
> Does it true for all cases? E.g., execCommand().

Ah. You're right that execCommand fires input in WebKit and Gecko. That seems fine to me. execCommand is a weird, crazy API. It's fine for it to behave a bit weird. The most common use of execCommand is to map it to toolbar buttons that the user presses anyways.

> > > The important difference between beforeinput and input is, beforeinput is
> > > cancellable. I.e., it MUST be fired before modifying editor value. This
> > > means that beforeinput may be nested if beforeinput handler causes another
> > > beforeinput. (input event isn't cancellable, therefore, browser can wait
> > > safe time to dispatch input event.)
> > 
> > I don't think it makes sense for beforeinput and input to fire for a
> > different set of actions.
> 
> beforeinput is used for cancelling user input. On the other hand, input is
> used for taking some reaction for user input. 

I mostly agree with this, except I don't think the *only* use-case for beforeinput is cancelling user input. Another use case is preparing for user input. For example, an undo manager could save the state before the user input.

> So, beforeinput only fired at
> caused by cancellable action does makes sense even if there are some cases
> input event is fired without beforeinput.

I don't agree with this conclusion. Partially because I think there are other use cases than cancelling the user input and partially because I think it's just confusing to developers.


Comment 6  Masayuki Nakano     2013-11-26 04:54:38 UTC  
(In reply to Ojan Vafai from comment #5)
> > > > The important difference between beforeinput and input is, beforeinput is
> > > > cancellable. I.e., it MUST be fired before modifying editor value. This
> > > > means that beforeinput may be nested if beforeinput handler causes another
> > > > beforeinput. (input event isn't cancellable, therefore, browser can wait
> > > > safe time to dispatch input event.)
> > > 
> > > I don't think it makes sense for beforeinput and input to fire for a
> > > different set of actions.
> > 
> > beforeinput is used for cancelling user input. On the other hand, input is
> > used for taking some reaction for user input. 
> 
> I mostly agree with this, except I don't think the *only* use-case for
> beforeinput is cancelling user input. Another use case is preparing for user
> input. For example, an undo manager could save the state before the user
> input.

I don't think so. The data attribute is too cheap for creating their own undo manager. Especially, removing selected text or a character before/after the caret.

> > So, beforeinput only fired at
> > caused by cancellable action does makes sense even if there are some cases
> > input event is fired without beforeinput.
> 
> I don't agree with this conclusion. Partially because I think there are
> other use cases than cancelling the user input and partially because I think
> it's just confusing to developers.

Then, how do you solve the security issue which is, beforeinput handler can cause another beforeinput?


Comment 7  Ojan Vafai     2013-11-27 21:33:07 UTC  
(In reply to Masayuki Nakano from comment #6)
> (In reply to Ojan Vafai from comment #5)
> > > > > The important difference between beforeinput and input is, beforeinput is
> > > > > cancellable. I.e., it MUST be fired before modifying editor value. This
> > > > > means that beforeinput may be nested if beforeinput handler causes another
> > > > > beforeinput. (input event isn't cancellable, therefore, browser can wait
> > > > > safe time to dispatch input event.)
> > > > 
> > > > I don't think it makes sense for beforeinput and input to fire for a
> > > > different set of actions.
> > > 
> > > beforeinput is used for cancelling user input. On the other hand, input is
> > > used for taking some reaction for user input. 
> > 
> > I mostly agree with this, except I don't think the *only* use-case for
> > beforeinput is cancelling user input. Another use case is preparing for user
> > input. For example, an undo manager could save the state before the user
> > input.
> 
> I don't think so. The data attribute is too cheap for creating their own
> undo manager. Especially, removing selected text or a character before/after
> the caret.

The point is the timing of the event. The event's properties are not used at all. It's a timing that fires *before* any user-initiated modification and lets you do whatever appropriate action you need to before the DOM is modified.

> > > So, beforeinput only fired at
> > > caused by cancellable action does makes sense even if there are some cases
> > > input event is fired without beforeinput.
> > 
> > I don't agree with this conclusion. Partially because I think there are
> > other use cases than cancelling the user input and partially because I think
> > it's just confusing to developers.
> 
> Then, how do you solve the security issue which is, beforeinput handler can
> cause another beforeinput?

How is it a security issue? How's it different from focus/blur events? You can get into the same sort of infinite loop with focus/blur events.


Comment 8  Masayuki Nakano     2013-11-27 23:50:36 UTC  
(In reply to Ojan Vafai from comment #7)
> > > > So, beforeinput only fired at
> > > > caused by cancellable action does makes sense even if there are some cases
> > > > input event is fired without beforeinput.
> > > 
> > > I don't agree with this conclusion. Partially because I think there are
> > > other use cases than cancelling the user input and partially because I think
> > > it's just confusing to developers.
> > 
> > Then, how do you solve the security issue which is, beforeinput handler can
> > cause another beforeinput?
> 
> How is it a security issue? How's it different from focus/blur events? You
> can get into the same sort of infinite loop with focus/blur events.

focus, focusin, focusout and blur are non-cancelable events. Therefore, web browser can delay to dispatch the events until previous event dispatching finishes. Actually, Gecko does this for focus, blur and input.

However, beforeinput is cancelable. Therefore, browser cannot delay to dispatch it.


Comment 9  Ojan Vafai     2013-11-28 00:21:41 UTC  
<div contentEditable id="foo">foo</div>
<div id="logger"></div>
<script>
var foo = document.getElementById('foo');
var logger = document.getElementById('logger');
foo.onfocus = function() {
  logger.innerHTML += 'focus started<br>';
  foo.blur();
  logger.innerHTML += 'focus done<br>';
};
foo.onblur = function() {
  logger.innerHTML += 'blur started<br>';
  foo.focus();
  logger.innerHTML += 'blur done<br>';
};
foo.focus();
</script>

Gecko prints out:
focus started
blur started
blur done
focus done

It doesn't hang, but the focus/blur events don't seem to be delayed either.

Incidentally, Blink prints out the same thing but 11 times, i.e:
focus started
blur started
focus started
blur started
...
blur done
focus done
blur done
focus done

I don't see why beforeinput can't get the same treatment.


Comment 10  Masayuki Nakano     2013-11-28 02:45:28 UTC  
I don't understand the Gecko's behavior perfectly, however, actually, Gecko has a mechanism to wait to dispatch some events until becoming "safe to dispatch". focus/blur events are dispatched via the mechanism:

http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp?rev=e864bbc290f5#1874


Comment 11  Masayuki Nakano     2013-11-28 02:47:04 UTC  
(In reply to Ojan Vafai from comment #9)
> I don't see why beforeinput can't get the same treatment.

Because such behavior may cause stack overflow (SpiderMonkey has a protect system for it, though).


Comment 12  Ojan Vafai     2013-12-02 23:35:28 UTC  
How about we change beforeInput and input to not fire for execCommand? They would still fire for user-initiated cut, paste, etc.


Comment 13  Masayuki Nakano     2013-12-10 07:52:29 UTC  
(In reply to Ojan Vafai from comment #12)
> How about we change beforeInput and input to not fire for execCommand? They
> would still fire for user-initiated cut, paste, etc.

I don't think it makes sense to prevent firing input event in such cases because web apps must expect input event is fired for every modifying the content. If not, it's not useful for implementing autocomplete/suggest list.

beforeinput is designed for an alternative event of keypress event. Therefore, it's cancellable and it causes different issues from input event.


Comment 14  Ojan Vafai     2013-12-10 23:32:01 UTC  
(In reply to Masayuki Nakano from comment #13)
> (In reply to Ojan Vafai from comment #12)
> > How about we change beforeInput and input to not fire for execCommand? They
> > would still fire for user-initiated cut, paste, etc.
> 
> I don't think it makes sense to prevent firing input event in such cases
> because web apps must expect input event is fired for every modifying the
> content. If not, it's not useful for implementing autocomplete/suggest list.

The input event already does not fire if you modify the content via DOM APIs (e.g. value or appendChild). The fact that it fires for execCommand seems like an accident of history to me.


Comment 15  Gary Kacmarcik     2014-04-08 01:55:16 UTC  
Summary to see if we have a consensus here:

This bug seems to be interpreting 'beforeinput' solely in the context as a replacement for 'keypress', but that was not the intent of 'beforeinput'. It was introduced as a way to trap input changes (from keyboard or other input device) before they were committed to the DOM (i.e., before the 'input' event). It was noticed that 'beforeinput' fired whenever 'keypress' would have fired, so 'keypress' was deemed redundant and removed (in the interest of simplifying the number of events flying around).  Hence, 'beforeinput' is the suggested replacement for 'keypress'.


Ojan proposed that we specify 'beforeinput' and 'input' to not fire for execCommand so we can avoid the problems mentioned in this bug.

This seems reasonable to me.

Any further comments/suggestions?


Comment 16  Masayuki Nakano     2014-04-16 01:16:21 UTC  
I think that "input" should be fired even when beforeinput is not fired because input event is always fired on current browser and it can be dispatched when it becomes safe.

"beforeinput"'s problem is caused by that it's a cancelable event.

If "beforeinput" should always be fired before "input", I'd like to suggest that "beforeinput" MAY be non-cencelable if it cannot be dispatched before actual DOM change due to unsafe caused by something. This might make web apps implementation simpler.

Please note that when "beforeinput" isn't cancelable due to unsafe, the change is caused by web apps, not normal text input by users (caused by key, IME and paste).

It might be unsafe when editor is changed by execCommand, setting .value, using innerHTML or something to contenteditable editor.


Comment 17  Gary Kacmarcik     2014-04-23 01:11:12 UTC  
In 5.2.7.6 "Input Events During Composition", we already have a note:

"Note: Some IMEs update the DOM before the compositionend event is dispatched. In this case, canceling the beforeinput event will have no effect (i.e., the input input will still fire)."

We may want to make it more obvious that 'input' may fire even if 'beforeinput' is cancelled. But we do want browsers to make a best-effort here.


Comment 18  Masayuki Nakano     2014-04-23 02:07:06 UTC  
The cause isn't IME, changing the editor contents from web apps...


Comment 19  Travis Leithead [MSFT]     2014-05-23 22:57:50 UTC  
So, I've lost track of what we want to do with this bug.

Our spec text, as currently stands, is very light on details. Of beforeinput, relative to the comments in the bug it says:

  ["Input events are sent as notifications whenever the DOM is being updated"]

A generalization: may not be strictly true if beforeinput/input are not dispatched when the DOM was modified by script.

  ["The input events defined in this specification MUST occur in a set order relative to one another."]

A generalization: basically true.

  ["A user agent MUST dispatch this event [beforeinput] when the DOM is about to be updated."]

May need to refine this text to explain the conditions under which is may not be dispatched or may be delayed, depending on what we do with Bug 25683.

  ["A user agent MUST dispatch this event [input] immediately after the DOM has been updated"]

Again, may need to clarify the conditions based on reality in user agents today.

Finally, as Gary noted above, we have 5.2.7.6, which talks about the order of events in relation to compositionupdate and compositionend.

  ["Most IMEs do not support canceling updates during a composition session."]

Does the above also mean that beforeinput isn't cancelable? I think it does.

  ["... In this case, canceling the beforeinput event will have no effect"]

This also seems to indicate that when an IME is in control the user agent may not have an opportunity to cancel the DOM updates, meaning that beforeinput can only be useful as a pre-change notification and not cancelable.

Is it still worth it to ask user agents to dispatch beforeinput in those last two cases (where they can't reasonably stop the IME's input)? If so, then beforeinput seems totally redundant with compositionupdate and compositionend at least in this scenario. I understand wanting to use beforeinput as a general catch all for user-input (including IME) and so perhaps it is OK to redundantly dispatch it, but we'll need to have a short section defining what it means to dispatch a non-cancelable beforeinput event. E.g., the cancelable attribute will be set to false, etc.


Comment 20  Yoshifumi Inoue     2014-06-12 07:30:03 UTC  
Do we think suggested word inserting as user input from context menu on misspell word right click?


---
Reply to this email directly or view it on GitHub:
https://github.com/w3c/editing-explainer/issues/43

Received on Wednesday, 21 January 2015 17:58:19 UTC