w3c / input-events

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

Declare that `dataTransfer` should be read-only unless created by JS #95

Closed masayuki-nakano closed 5 years ago

masayuki-nakano commented 5 years ago

Currently, there is no detail of dataTransfer attribute value of input nor beforeinput event.

At least I've tested, Chrome sets dataTransfer to read-only DataTransfer instance. For example, clearData() does not clear its data. I think that this is reasonable behavior because if we need to allow to modify the DataTransfer instance, editor of browser needs to copy the instance every time. This causes editor performance too bad since memory allocation is too slow but source of DataTransfer may be too big.

On the other hand, if the InputEvent instance is created by JS, whether the DataTransfer object is read-only, read-write or protected should depend on the DataTransfer instance which is set via the dictionary at constructing. I.e., when InputEvent is created with a DataTransfer instance, InputEvent should set its dataTransfer to the instance itself. Then, InputEvent does not need to clone given DataTransfer instance.

johanneswilm commented 5 years ago

@masayuki-nakano If this makes it easier to work with for browsers, I agree. I cannot think of any reason why JS would want to modify anything about these browser initiated events.

johanneswilm commented 5 years ago

@masayuki-nakano Does it not specify in the webidl that it should be readonly? https://w3c.github.io/input-events/#interface-InputEvent Where else would you expect it?

masayuki-nakano commented 5 years ago

@johanneswilm I meant that we shouldn't make InputEvent.dataTransfer read-only. I meant that spec shouldn't allow this code:

let dataTransfer = event.dataTransfer;
dataTransfer.setData("text/plain", "customized paste data");

If spec allows this, "input" and "beforeinput" event dispatcher of browser needs to clone dataTransfer every time because coming transferable data may not be allowed to modify. If the transferable data is too big (e.g., several MB text), browser may need to allocate same space and copy such big data. Of course, browsers can optimize this kind of things (e.g., allocating when first modification). However, if nobody wants to do this, not allowing this makes browser implementations simpler and faster (at run-time performance).

FYI: I added this testcase into the WPT: https://searchfox.org/mozilla-central/rev/ddd1679c0534f7ddf36cafddd17b710c4fefe3c4/testing/web-platform/tests/input-events/input-events-exec-command.html#76-93

johanneswilm commented 5 years ago

@masayuki-nakano Ok, so we need a variant of Datatransfer where the setData- and clearData-methods aren't working. Basically we need the drag data store mode to be Read-only [1], right?

[1] https://html.spec.whatwg.org/multipage/dnd.html#drag-data-store-mode

masayuki-nakano commented 5 years ago

@johanneswilm Yeah, I think that Read-only is what it should be here. (Although I'm not familiar with the detail of DataTransfer.) See here, in read-only mode, setData() nor clearData() does nothing.