This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 25683 - Document "unsafe to dispatch events" in D3E if it's possible
Summary: Document "unsafe to dispatch events" in D3E if it's possible
Status: RESOLVED MOVED
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: HISTORICAL - DOM3 Events (show other bugs)
Version: unspecified
Hardware: All All
: P2 enhancement
Target Milestone: ---
Assignee: Travis Leithead [MSFT]
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 23913
  Show dependency treegraph
 
Reported: 2014-05-13 12:26 UTC by Masayuki Nakano
Modified: 2015-01-21 20:38 UTC (History)
6 users (show)

See Also:


Attachments

Description Masayuki Nakano 2014-05-13 12:26:16 UTC
I've commented about "unsafe to dispatch events" case in bug 23913.

Similar issue as bug 23913 exists with focusin, focusout and DOMNodeRemoved since they and beforeinput are defined the timing to be dispatched by D3E.

According to Olli, while content is changing, it's unsafe to dispatch DOM events because rendering engine cannot handle changes at random times. An event listener may change DOM or close windows or whatever.

"unsafe" means that no unexpected DOM tree or layout changes (or other changes, like new page loads etc). In general it means that if scripts would run during that time, it would most probably leads to sec-critical bugs.

So, I'd like to suggest that unloading document, changing DOM tree or changing editor content should be defined as "may be unsafe". If a browser needs to dispatch an event but it's impossible due to unsafe state, D3E should allow browsers to dispatch events immediately after becoming safe. Then, cancelable event may not be able to prevent default action.
Comment 1 Olli Pettay 2014-05-13 12:32:13 UTC
"unsafeness" is implementation detail, but DOMNodeRemoved has, AFAIK, 
caused some pain to all the implementations.

I don't think the spec should have unsafeness defined.
We should perhaps avoid adding new DOMNodeRemoved-type of events, but
implementations should need to deal with the unsafeness of the existing
events.
Comment 2 Anne 2014-05-13 12:51:24 UTC
We should not define mutation events at all I think as we're still planning on killing them.

We should be clearer about how events interact with the event loop.
Comment 3 Olli Pettay 2014-05-13 13:07:37 UTC
(In reply to Anne from comment #2)
> We should not define mutation events at all I think as we're still planning
> on killing them.
Sure, but there are other tricky (tricky at least in some implementations) events


> 
> We should be clearer about how events interact with the event loop.
That doesn't really have much to do with unsafeness stuff.
Comment 4 Travis Leithead [MSFT] 2014-05-23 22:19:43 UTC
Rather than document the concept of unsafe, I like the approach that HTML5 took with focus management, which can be seen here:

http://www.w3.org/html/wg/drafts/html/master/single-page.html#dom-focus

The idea is that each element has a "locked for focus" flag. The flag is checked before focus is switched (this is presumably how focusin and focusout are made safe from the recursion--though HTML5 doesn't mention these events in the focus update steps (http://www.w3.org/html/wg/drafts/html/master/single-page.html#focus-update-steps). 

(Filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=25877 on HTML5 to update to include these two.)

So, for beforeinput, we could use the same tactic

1. If target of the beforeinput event is beforeinput-locked, then abort;
2. Make the target of beforeinput event "beforeinput-locked";
3. Dispatch beforeinput;
4. Clear the "beforeinput-locked" flag on the target;
4. If beforeinput was canceled, then abort;
5. Update DOM (with content as described by beforeinput event)

This algorithm basically just eliminates the recursion (rather than allowing the event to be randomly delayed and made non-cancelable as a result).

If we allow execCommand to generate beforeinput, then the following scenario could occur:

1. User Types "A" into input box "input"
2. (keydown event dispatched at "input" and is not cancelled)
3. beforeinput event dispatched at "input"
 3.1. Event handler for beforeinput event invoked
 3.2. Event handler calls execCommand("insertText", .. "B") (on "input")
 3.3. (see below)
 3.4. Event handler cancels the beforeinput event

In conclusion, the "input" has a value of "B".

In step 3.3., the beforeinput event might have been dispatched, but the "beforeinput-locked" flag prevents it from firing. This is actually good because this was a script-initiated insertion (synchronously performed) which the developer would otherwise need to ignore in a recursively-invoked beforeinput event handler anyway. A similar situation would arise if the "input" target's value attribute was modified in the handler (assuming that beforeinput fired for script-initiated changes too--which I don't think we want to enable). I can't think of how to force a copy/paste operation (outside of execCommand) that is synchronous, but if it can be done, this algorithm protects that too.

Also note: the algorithm only locks one element at a time, so it's possible to have "input transfer" scenarios, where your beforeinput handler catches the input in order to redirect it into another input box or contenteditable.
Comment 5 Masayuki Nakano 2014-05-25 03:05:52 UTC
(In reply to Travis Leithead [MSFT] from comment #4)
> 1. If target of the beforeinput event is beforeinput-locked, then abort;

If browsers do so, it could break compatibility with some browsers which don't support "beforeinput" since such browsers don't need to check it and may allow text input at that time.


> If we allow execCommand to generate beforeinput, then the following scenario
> could occur:
> 
> 1. User Types "A" into input box "input"
> 2. (keydown event dispatched at "input" and is not cancelled)
> 3. beforeinput event dispatched at "input"
>  3.1. Event handler for beforeinput event invoked
>  3.2. Event handler calls execCommand("insertText", .. "B") (on "input")
>  3.3. (see below)
>  3.4. Event handler cancels the beforeinput event
> 
> In conclusion, the "input" has a value of "B".
> 
> In step 3.3., the beforeinput event might have been dispatched, but the
> "beforeinput-locked" flag prevents it from firing. This is actually good
> because this was a script-initiated insertion (synchronously performed)
> which the developer would otherwise need to ignore in a recursively-invoked
> beforeinput event handler anyway. A similar situation would arise if the
> "input" target's value attribute was modified in the handler (assuming that
> beforeinput fired for script-initiated changes too--which I don't think we
> want to enable). I can't think of how to force a copy/paste operation
> (outside of execCommand) that is synchronous, but if it can be done, this
> algorithm protects that too.

This sound good.

> 
> Also note: the algorithm only locks one element at a time, so it's possible
> to have "input transfer" scenarios, where your beforeinput handler catches
> the input in order to redirect it into another input box or contenteditable.

Did you mean that "beforeinput-locked" is document or window wide state?
Comment 6 Travis Leithead [MSFT] 2014-05-28 01:04:43 UTC
(In reply to Masayuki Nakano from comment #5)
> (In reply to Travis Leithead [MSFT] from comment #4)
> > 1. If target of the beforeinput event is beforeinput-locked, then abort;
> 
> If browsers do so, it could break compatibility with some browsers which
> don't support "beforeinput" since such browsers don't need to check it and
> may allow text input at that time.

A valid concern; I don't believe anyone implements beforeinput yet, so these behavioral differences may not be a concern. If browsers add it, yes it could change behavior for users, but adds the functionality.


> > Also note: the algorithm only locks one element at a time, so it's possible
> > to have "input transfer" scenarios, where your beforeinput handler catches
> > the input in order to redirect it into another input box or contenteditable.
> 
> Did you mean that "beforeinput-locked" is document or window wide state?

I expect the "beforeinput-locked" flag to be per-element.
Comment 7 Ben Peters 2015-01-21 17:59:43 UTC
Copied to https://github.com/w3c/editing-explainer/issues/44

Please continue the conversation there.
Comment 8 Travis Leithead [MSFT] 2015-01-21 20:38:33 UTC
Bug has moved to Github issue tracking for Editing TF current work.