Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix onPaste handler to work with copying files from Finder #5389

Merged
merged 3 commits into from
Jul 22, 2021
Merged

Fix onPaste handler to work with copying files from Finder #5389

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee8d1f5
Fix onPaste handler to work with copying files from Finder
t3chguy Nov 3, 2020
7a64adf
Merge branch 'develop' of github.com:matrix-org/matrix-react-sdk into…
t3chguy Jul 16, 2021
e600787
post-merge fixup
t3chguy Jul 16, 2021
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
12 changes: 5 additions & 7 deletions src/components/views/rooms/SendMessageComposer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,13 +445,11 @@ export default class SendMessageComposer extends React.Component {

_onPaste = (event) => {
const {clipboardData} = event;
// Prioritize text on the clipboard over files as Office on macOS puts a bitmap
// in the clipboard as well as the content being copied.
if (clipboardData.files.length && !clipboardData.types.some(t => t === "text/plain")) {
// This actually not so much for 'files' as such (at time of writing
// neither chrome nor firefox let you paste a plain file copied
// from Finder) but more images copied from a different website
// / word processor etc.
// Prioritize text on the clipboard over files if RTF is present as Office on macOS puts a bitmap
// in the clipboard as well as the content being copied. Modern versions of Office seem to not do this anymore.
Copy link
Member

Choose a reason for hiding this comment

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

How modern is modern? As recently as this summer it was still broken.

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea, but this is still safe from it as per your comment on the original issue that Office version also put text/rtf in it

// We check text/rtf instead of text/plain as when copy+pasting a file from Finder it puts the filename
// in as text/plain which we want to ignore.
if (clipboardData.files.length && !clipboardData.types.includes("text/rtf")) {
Copy link
Member

Choose a reason for hiding this comment

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

Surely we should still be falling back to text/plain too?

Copy link
Member Author

Choose a reason for hiding this comment

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

as the comment says, this heuristic is based on rtf because Finder puts both the File and test/plain with the filename in it. Office puts text/rtf text/html text/plain and in some prior version also a bitmap

Copy link
Member

Choose a reason for hiding this comment

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

but we're not dealing with just Apple here, this is also a thing for Windows and Linux, though the Office issue in particular is actually documented.

Copy link
Member Author

Choose a reason for hiding this comment

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

The behaviour should be to use the file 1st except in the case of Office/other known faulty things to enable copy pasting files where an annotation of the file name is also attached.

Copy link
Member

Choose a reason for hiding this comment

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

yes, and the point I'm trying to make is that not all of those things publish text/rtf.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you give an example of one so I can see how else to work around it? Do you have any suggestions on how to work around them whilst fixing the regression of breaking copy paste from Finder?

Copy link
Member Author

Choose a reason for hiding this comment

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

Based on your original report text/rtf was present

element-hq/element-web#13985 (comment)

ContentMessages.sharedInstance().sendContentListToRoom(
Array.from(clipboardData.files), this.props.room.roomId, this.context,
);
Expand Down