Skip to content
Merged
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/web/waiters/InputWaiter.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,18 @@ class InputWaiter {
// Event handlers
EditorView.domEventHandlers({
paste(event, view) {
const clipboardData = event.clipboardData || window.clipboardData;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@t-martine what is the purpose of window.clipboardData here? I cannot find any documentation for that property.
ClipboardEvent.clipboardData is baseline widely available, so I would have thought it is fine to just use that: https://developer.mozilla.org/en-US/docs/Web/API/ClipboardEvent/clipboardData#browser_compatibility

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was meant for backwards compatibility with IE11 but I agree to remove it since IE is officially out of support for a while now.

https://stackoverflow.com/questions/56063191/javascript-code-for-copy-to-clipboard-wont-work-in-internet-explorer-11

const items = clipboardData.items;
event.target.files = [];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like bad practice to me to add a custom attribute to an external type. Couldn't we create this list as a local variable, and update the signature of afterPaste to accept a list of files as a second parameter?

for (let i = 0; i < items.length; i++) {
const item = items[i];
if (item.type.indexOf("image") !== -1) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good if we could support all file types, rather than just images. Could you not instead check item.kind === 'file'?
https://developer.mozilla.org/en-US/docs/Web/API/DataTransferItem/kind

const file = item.getAsFile();
event.target.files.push(file);

event.preventDefault(); // Prevent the default paste behavior
}
}
setTimeout(() => {
self.afterPaste(event);
});
Expand Down Expand Up @@ -917,6 +929,9 @@ class InputWaiter {
* @param {event} e
*/
afterPaste(e) {
if (e.target.files.length > 0) {
this.loadUIFiles(e.target.files);
}
// If EOL has been fixed, skip this.
if (this.eolState > 1) return;

Expand Down