From 563fefaadd03f2bac61eefa1cad62b7252b8bf58 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 23 Mar 2023 13:39:54 +0000 Subject: [PATCH 01/15] add comments to rough first solution --- .../hooks/useInputEventProcessor.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index 9b41227ba3b..2801fd7d014 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -34,6 +34,7 @@ import { getEventsFromEditorStateTransfer, getEventsFromRoom } from "../utils/ev import { endEditing } from "../utils/editing"; import Autocomplete from "../../Autocomplete"; import { handleEventWithAutocomplete } from "./utils"; +import ContentMessages from "../../../../../ContentMessages"; export function useInputEventProcessor( onSend: () => void, @@ -48,6 +49,29 @@ export function useInputEventProcessor( return useCallback( (event: WysiwygEvent, composer: Wysiwyg, editor: HTMLElement) => { if (event instanceof ClipboardEvent) { + // This code unashamedly pillaged from SendMessageComposer.onPaste with the arguments + // tweaked a bit to see if it works. + // It does work, just need to figure out the `this.props.relation` translation + + const { clipboardData } = event; + if (clipboardData?.files.length && !clipboardData.types.includes("text/rtf")) { + if (roomContext.room?.roomId) { + ContentMessages.sharedInstance().sendContentListToRoom( + Array.from(clipboardData.files), + roomContext.room.roomId, + {}, // this.props.relation, is IEventRelation, which is passed through MessageComposer + // which it looks like comes from ThreadView + // looks like there is a call to this.props.room.getThread(this.props.mxEvent.getId()) + + // looks like skipping out this bit means when you try and paste into a thread in the right + // panel, it actually just appears in the timeline + mxClient, + roomContext.timelineRenderingType, + ); + return null; + } + } + return event; } From ac89c45117d5b05e460f412349caf139fcf04e0d Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 6 Jun 2023 11:45:33 +0100 Subject: [PATCH 02/15] allow eventRelation prop to pass to both composers --- .../views/rooms/wysiwyg_composer/SendWysiwygComposer.tsx | 3 +-- .../rooms/wysiwyg_composer/components/PlainTextComposer.tsx | 2 ++ .../rooms/wysiwyg_composer/components/WysiwygComposer.tsx | 5 ++++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/SendWysiwygComposer.tsx b/src/components/views/rooms/wysiwyg_composer/SendWysiwygComposer.tsx index d12432481db..34307ce4abd 100644 --- a/src/components/views/rooms/wysiwyg_composer/SendWysiwygComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/SendWysiwygComposer.tsx @@ -57,11 +57,10 @@ export default function SendWysiwygComposer({ isRichTextEnabled, e2eStatus, menuPosition, - eventRelation, ...props }: SendWysiwygComposerProps): JSX.Element { const Composer = isRichTextEnabled ? WysiwygComposer : PlainTextComposer; - const defaultContextValue = useRef(getDefaultContextValue({ eventRelation })); + const defaultContextValue = useRef(getDefaultContextValue({ eventRelation: props.eventRelation })); return ( diff --git a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx index c6abc1230b4..efc4971657d 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/PlainTextComposer.tsx @@ -15,6 +15,7 @@ limitations under the License. */ import classNames from "classnames"; +import { IEventRelation } from "matrix-js-sdk/src/matrix"; import React, { MutableRefObject, ReactNode } from "react"; import { useComposerFunctions } from "../hooks/useComposerFunctions"; @@ -36,6 +37,7 @@ interface PlainTextComposerProps { leftComponent?: ReactNode; rightComponent?: ReactNode; children?: (ref: MutableRefObject, composerFunctions: ComposerFunctions) => ReactNode; + eventRelation?: IEventRelation; } export function PlainTextComposer({ diff --git a/src/components/views/rooms/wysiwyg_composer/components/WysiwygComposer.tsx b/src/components/views/rooms/wysiwyg_composer/components/WysiwygComposer.tsx index 56f79c94a87..c4c50b8392c 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/WysiwygComposer.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/WysiwygComposer.tsx @@ -15,6 +15,7 @@ limitations under the License. */ import React, { memo, MutableRefObject, ReactNode, useEffect, useRef } from "react"; +import { IEventRelation } from "matrix-js-sdk/src/matrix"; import { useWysiwyg, FormattingFunctions } from "@matrix-org/matrix-wysiwyg"; import classNames from "classnames"; @@ -40,6 +41,7 @@ interface WysiwygComposerProps { leftComponent?: ReactNode; rightComponent?: ReactNode; children?: (ref: MutableRefObject, wysiwyg: FormattingFunctions) => ReactNode; + eventRelation?: IEventRelation; } export const WysiwygComposer = memo(function WysiwygComposer({ @@ -52,11 +54,12 @@ export const WysiwygComposer = memo(function WysiwygComposer({ leftComponent, rightComponent, children, + eventRelation, }: WysiwygComposerProps) { const { room } = useRoomContext(); const autocompleteRef = useRef(null); - const inputEventProcessor = useInputEventProcessor(onSend, autocompleteRef, initialContent); + const inputEventProcessor = useInputEventProcessor(onSend, autocompleteRef, initialContent, eventRelation); const { ref, isWysiwygReady, content, actionStates, wysiwyg, suggestion } = useWysiwyg({ initialContent, inputEventProcessor, From eddb25a0d8691aa819b6edd14b765d41fc5a0386 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 6 Jun 2023 11:45:49 +0100 Subject: [PATCH 03/15] use eventRelation in image paste --- .../hooks/useInputEventProcessor.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index 2801fd7d014..3c36b09d0a4 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -16,7 +16,7 @@ limitations under the License. import { Wysiwyg, WysiwygEvent } from "@matrix-org/matrix-wysiwyg"; import { useCallback } from "react"; -import { MatrixClient } from "matrix-js-sdk/src/matrix"; +import { IEventRelation, MatrixClient } from "matrix-js-sdk/src/matrix"; import { useSettingValue } from "../../../../../hooks/useSettings"; import { getKeyBindingsManager } from "../../../../../KeyBindingsManager"; @@ -40,6 +40,7 @@ export function useInputEventProcessor( onSend: () => void, autocompleteRef: React.RefObject, initialContent?: string, + eventRelation?: IEventRelation, ): (event: WysiwygEvent, composer: Wysiwyg, editor: HTMLElement) => WysiwygEvent | null { const roomContext = useRoomContext(); const composerContext = useComposerContext(); @@ -59,7 +60,8 @@ export function useInputEventProcessor( ContentMessages.sharedInstance().sendContentListToRoom( Array.from(clipboardData.files), roomContext.room.roomId, - {}, // this.props.relation, is IEventRelation, which is passed through MessageComposer + eventRelation, + // this.props.relation, is IEventRelation, which is passed through MessageComposer // which it looks like comes from ThreadView // looks like there is a call to this.props.room.getThread(this.props.mxEvent.getId()) @@ -102,7 +104,16 @@ export function useInputEventProcessor( return handleInputEvent(event, send, isCtrlEnterToSend); } }, - [isCtrlEnterToSend, onSend, initialContent, roomContext, composerContext, mxClient, autocompleteRef], + [ + isCtrlEnterToSend, + onSend, + initialContent, + roomContext, + composerContext, + mxClient, + autocompleteRef, + eventRelation, + ], ); } From 5193f2b6386965e846ae01cd111a34e474ac01df Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 6 Jun 2023 12:55:50 +0100 Subject: [PATCH 04/15] add image pasting to rich text mode of rich text editor --- .../hooks/useInputEventProcessor.ts | 125 ++++++++++++++---- 1 file changed, 97 insertions(+), 28 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index 3c36b09d0a4..f3f837cac97 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -35,6 +35,7 @@ import { endEditing } from "../utils/editing"; import Autocomplete from "../../Autocomplete"; import { handleEventWithAutocomplete } from "./utils"; import ContentMessages from "../../../../../ContentMessages"; +import { getBlobSafeMimeType } from "../../../../../utils/blobs"; export function useInputEventProcessor( onSend: () => void, @@ -49,34 +50,6 @@ export function useInputEventProcessor( return useCallback( (event: WysiwygEvent, composer: Wysiwyg, editor: HTMLElement) => { - if (event instanceof ClipboardEvent) { - // This code unashamedly pillaged from SendMessageComposer.onPaste with the arguments - // tweaked a bit to see if it works. - // It does work, just need to figure out the `this.props.relation` translation - - const { clipboardData } = event; - if (clipboardData?.files.length && !clipboardData.types.includes("text/rtf")) { - if (roomContext.room?.roomId) { - ContentMessages.sharedInstance().sendContentListToRoom( - Array.from(clipboardData.files), - roomContext.room.roomId, - eventRelation, - // this.props.relation, is IEventRelation, which is passed through MessageComposer - // which it looks like comes from ThreadView - // looks like there is a call to this.props.room.getThread(this.props.mxEvent.getId()) - - // looks like skipping out this bit means when you try and paste into a thread in the right - // panel, it actually just appears in the timeline - mxClient, - roomContext.timelineRenderingType, - ); - return null; - } - } - - return event; - } - const send = (): void => { event.stopPropagation?.(); event.preventDefault?.(); @@ -87,6 +60,11 @@ export function useInputEventProcessor( onSend(); }; + const isClipboardEvent = event instanceof ClipboardEvent; + if (isClipboardEvent) { + return handleClipboardEvent(event, roomContext, mxClient, eventRelation); + } + const isKeyboardEvent = event instanceof KeyboardEvent; if (isKeyboardEvent) { return handleKeyboardEvent( @@ -255,3 +233,94 @@ function handleInputEvent(event: InputEvent, send: Send, isCtrlEnterToSend: bool return event; } + +/** + * Takes a ClipboardEvent and handles image pasting. If the event is not a paste event, or + * pasting is not successful, returns the original event to allow it to pass through. + * Otherwise, returns null to indicate that the event has been handled. + * + * @param clipboardEvent - event to process + * @param roomContext - room in which the event occurs + * @param mxClient - current matrix client + * @param eventRelation - used to send the event to the correct timeline + * @returns - null if event is handled here, the `clipboardEvent` param if not + */ +function handleClipboardEvent( + clipboardEvent: ClipboardEvent, + roomContext: IRoomState, + mxClient: MatrixClient, + eventRelation?: IEventRelation, +): ClipboardEvent | null { + const { clipboardData: data } = clipboardEvent; + const { room, timelineRenderingType, replyToEvent } = roomContext; + + if (clipboardEvent.type !== "paste" || data === null || room === undefined) { + return clipboardEvent; + } + + // 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. + // We check text/rtf instead of text/plain as when copy+pasting a file from Finder or Gnome Image Viewer + // it puts the filename in as text/plain which we want to ignore. + if (data.files.length && !data.types.includes("text/rtf")) { + ContentMessages.sharedInstance().sendContentListToRoom( + Array.from(data.files), + room.roomId, + eventRelation, + mxClient, + timelineRenderingType, + ); + return null; + } + + // Safari `Insert from iPhone or iPad` + // data.getData("text/html") returns a string like: + if (data.types.includes("text/html")) { + const imgElementStr = data.getData("text/html"); + const parser = new DOMParser(); + const imgDoc = parser.parseFromString(imgElementStr, "text/html"); + + if ( + imgDoc.getElementsByTagName("img").length !== 1 || + !imgDoc.querySelector("img")?.src.startsWith("blob:") || + imgDoc.childNodes.length !== 1 + ) { + console.log("Failed to handle pasted content as Safari inserted content"); + return clipboardEvent; + } + const imgSrc = imgDoc!.querySelector("img")!.src; + + fetch(imgSrc).then( + (response) => { + response.blob().then( + (imgBlob) => { + const type = imgBlob.type; + const safetype = getBlobSafeMimeType(type); + const ext = type.split("/")[1]; + const parts = response.url.split("/"); + const filename = parts[parts.length - 1]; + const file = new File([imgBlob], filename + "." + ext, { type: safetype }); + ContentMessages.sharedInstance().sendContentToRoom( + file, + room.roomId, + eventRelation, + mxClient, + replyToEvent, + ); + return null; + }, + (error) => { + console.log(error); + return clipboardEvent; + }, + ); + }, + (error) => { + console.log(error); + return clipboardEvent; + }, + ); + } + + return clipboardEvent; +} From 5cc2442c009fdab869798db40e29a7991cf4dded Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 6 Jun 2023 13:39:20 +0100 Subject: [PATCH 05/15] extract error handling to function --- .../hooks/useInputEventProcessor.ts | 64 ++++++++----------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index f3f837cac97..11784f49190 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -254,6 +254,11 @@ function handleClipboardEvent( const { clipboardData: data } = clipboardEvent; const { room, timelineRenderingType, replyToEvent } = roomContext; + function handleError(error): ClipboardEvent { + console.log(error); + return clipboardEvent; + } + if (clipboardEvent.type !== "paste" || data === null || room === undefined) { return clipboardEvent; } @@ -263,13 +268,9 @@ function handleClipboardEvent( // We check text/rtf instead of text/plain as when copy+pasting a file from Finder or Gnome Image Viewer // it puts the filename in as text/plain which we want to ignore. if (data.files.length && !data.types.includes("text/rtf")) { - ContentMessages.sharedInstance().sendContentListToRoom( - Array.from(data.files), - room.roomId, - eventRelation, - mxClient, - timelineRenderingType, - ); + ContentMessages.sharedInstance() + .sendContentListToRoom(Array.from(data.files), room.roomId, eventRelation, mxClient, timelineRenderingType) + .catch(handleError); return null; } @@ -285,41 +286,28 @@ function handleClipboardEvent( !imgDoc.querySelector("img")?.src.startsWith("blob:") || imgDoc.childNodes.length !== 1 ) { - console.log("Failed to handle pasted content as Safari inserted content"); - return clipboardEvent; + handleError("Failed to handle pasted content as Safari inserted content"); } const imgSrc = imgDoc!.querySelector("img")!.src; - fetch(imgSrc).then( - (response) => { - response.blob().then( - (imgBlob) => { - const type = imgBlob.type; - const safetype = getBlobSafeMimeType(type); - const ext = type.split("/")[1]; - const parts = response.url.split("/"); - const filename = parts[parts.length - 1]; - const file = new File([imgBlob], filename + "." + ext, { type: safetype }); - ContentMessages.sharedInstance().sendContentToRoom( - file, - room.roomId, - eventRelation, - mxClient, - replyToEvent, - ); - return null; - }, - (error) => { - console.log(error); - return clipboardEvent; - }, + fetch(imgSrc).then((response) => { + response.blob().then((imgBlob) => { + const type = imgBlob.type; + const safetype = getBlobSafeMimeType(type); + const ext = type.split("/")[1]; + const parts = response.url.split("/"); + const filename = parts[parts.length - 1]; + const file = new File([imgBlob], filename + "." + ext, { type: safetype }); + ContentMessages.sharedInstance().sendContentToRoom( + file, + room.roomId, + eventRelation, + mxClient, + replyToEvent, ); - }, - (error) => { - console.log(error); - return clipboardEvent; - }, - ); + return null; + }, handleError); + }, handleError); } return clipboardEvent; From 1853439046fe701f0bb3c1f71900648f3b62bc6e Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 6 Jun 2023 14:33:33 +0100 Subject: [PATCH 06/15] type the error handler --- .../hooks/useInputEventProcessor.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index 11784f49190..3c7316f84bf 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -254,8 +254,12 @@ function handleClipboardEvent( const { clipboardData: data } = clipboardEvent; const { room, timelineRenderingType, replyToEvent } = roomContext; - function handleError(error): ClipboardEvent { - console.log(error); + function errorHandler(error: unknown): ClipboardEvent { + if (error instanceof Error) { + console.log(error.message); + } else if (error instanceof String) { + console.log(error); + } return clipboardEvent; } @@ -270,7 +274,7 @@ function handleClipboardEvent( if (data.files.length && !data.types.includes("text/rtf")) { ContentMessages.sharedInstance() .sendContentListToRoom(Array.from(data.files), room.roomId, eventRelation, mxClient, timelineRenderingType) - .catch(handleError); + .catch(errorHandler); return null; } @@ -286,7 +290,7 @@ function handleClipboardEvent( !imgDoc.querySelector("img")?.src.startsWith("blob:") || imgDoc.childNodes.length !== 1 ) { - handleError("Failed to handle pasted content as Safari inserted content"); + errorHandler("Failed to handle pasted content as Safari inserted content"); } const imgSrc = imgDoc!.querySelector("img")!.src; @@ -306,8 +310,8 @@ function handleClipboardEvent( replyToEvent, ); return null; - }, handleError); - }, handleError); + }, errorHandler); + }, errorHandler); } return clipboardEvent; From ff2256643807755f990bb409eec5067aa38d3f21 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 6 Jun 2023 17:30:54 +0100 Subject: [PATCH 07/15] add tests --- .../hooks/useInputEventProcessor-test.tsx | 220 ++++++++++++++++++ 1 file changed, 220 insertions(+) create mode 100644 test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx diff --git a/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx b/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx new file mode 100644 index 00000000000..84bda4d7edd --- /dev/null +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx @@ -0,0 +1,220 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +import { IEventRelation } from "matrix-js-sdk/src/matrix"; +import { waitFor } from "@testing-library/react"; + +import { handleClipboardEvent } from "../../../../../../src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor"; +import { TimelineRenderingType } from "../../../../../../src/contexts/RoomContext"; +import { Layout } from "../../../../../../src/settings/enums/Layout"; +import { mkStubRoom, stubClient } from "../../../../../test-utils"; +import ContentMessages from "../../../../../../src/ContentMessages"; + +describe("handleClipboardEvent", () => { + const mockClient = stubClient(); + const mockRoom = mkStubRoom("mock room", "mock room", mockClient); + const mockRoomState = { + room: mockRoom, + roomLoading: true, + peekLoading: false, + shouldPeek: true, + membersLoaded: false, + numUnreadMessages: 0, + canPeek: false, + showApps: false, + isPeeking: false, + showRightPanel: true, + joining: false, + showTopUnreadMessagesBar: false, + statusBarVisible: false, + canReact: false, + canSelfRedact: false, + canSendMessages: false, + resizing: false, + layout: Layout.Group, + lowBandwidth: false, + alwaysShowTimestamps: false, + showTwelveHourTimestamps: false, + readMarkerInViewThresholdMs: 3000, + readMarkerOutOfViewThresholdMs: 30000, + showHiddenEvents: false, + showReadReceipts: true, + showRedactions: true, + showJoinLeaves: true, + showAvatarChanges: true, + showDisplaynameChanges: true, + matrixClientIsReady: false, + showUrlPreview: false, + timelineRenderingType: TimelineRenderingType.Room, + threadId: undefined, + liveTimeline: undefined, + narrow: false, + activeCall: null, + msc3946ProcessDynamicPredecessor: false, + }; + + const sendContentListToRoomSpy = jest.spyOn(ContentMessages.sharedInstance(), "sendContentListToRoom"); + const sendContentToRoomSpy = jest.spyOn(ContentMessages.sharedInstance(), "sendContentToRoom"); + const fetchSpy = jest.spyOn(window, "fetch"); + const logSpy = jest.spyOn(console, "log").mockImplementation(() => {}); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + + it("returns false if it is not a paste event", () => { + const originalEvent = { type: "copy" } as ClipboardEvent; + const input = [originalEvent, mockRoomState, mockClient] as const; + const output = handleClipboardEvent(...input); + expect(output).toBe(false); + }); + + it("returns false if clipboard data is null", () => { + const originalEvent = { type: "paste", clipboardData: null } as ClipboardEvent; + const input = [originalEvent, mockRoomState, mockClient] as const; + const output = handleClipboardEvent(...input); + expect(output).toBe(false); + }); + + it("returns false if room is undefined", () => { + const originalEvent = { type: "paste" } as ClipboardEvent; + const { room, ...roomStateWithoutRoom } = mockRoomState; + const input = [originalEvent, roomStateWithoutRoom, mockClient] as const; + const output = handleClipboardEvent(...input); + expect(output).toBe(false); + }); + + it("handles event and calls sendContentListToRoom when data files are present", () => { + const originalEvent = { + type: "paste", + clipboardData: { files: ["something here"], types: [] }, + } as unknown as ClipboardEvent; + const input = [originalEvent, mockRoomState, mockClient] as const; + const output = handleClipboardEvent(...input); + + expect(sendContentListToRoomSpy).toHaveBeenCalledTimes(1); + expect(sendContentListToRoomSpy).toHaveBeenCalledWith( + originalEvent.clipboardData?.files, + mockRoom.roomId, + undefined, // this is the event relation, an optional arg + mockClient, + mockRoomState.timelineRenderingType, + ); + expect(output).toBe(true); + }); + + it("calls sendContentListToRoom with eventRelation when present", () => { + const originalEvent = { + type: "paste", + clipboardData: { files: ["something here"], types: [] }, + } as unknown as ClipboardEvent; + const mockEventRelation = {} as unknown as IEventRelation; + const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; + const output = handleClipboardEvent(...input); + + expect(sendContentListToRoomSpy).toHaveBeenCalledTimes(1); + expect(sendContentListToRoomSpy).toHaveBeenCalledWith( + originalEvent.clipboardData?.files, + mockRoom.roomId, + mockEventRelation, // this is the event relation, an optional arg + mockClient, + mockRoomState.timelineRenderingType, + ); + expect(output).toBe(true); + }); + + it("calls the error handler when sentContentListToRoom errors", async () => { + const mockErrorMessage = "something went wrong"; + sendContentListToRoomSpy.mockRejectedValueOnce(new Error(mockErrorMessage)); + + const originalEvent = { + type: "paste", + clipboardData: { files: ["something here"], types: [] }, + } as unknown as ClipboardEvent; + const mockEventRelation = {} as unknown as IEventRelation; + const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; + const output = handleClipboardEvent(...input); + + expect(sendContentListToRoomSpy).toHaveBeenCalledTimes(1); + await waitFor(() => { + expect(logSpy).toHaveBeenCalledWith(mockErrorMessage); + }); + expect(output).toBe(true); + }); + + it("calls the error handler when data types has text/html but data can not be parsed", () => { + const originalEvent = { + type: "paste", + clipboardData: { + files: [], + types: ["text/html"], + getData: jest.fn().mockReturnValue("
invalid html"), + }, + } as unknown as ClipboardEvent; + const mockEventRelation = {} as unknown as IEventRelation; + const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; + const output = handleClipboardEvent(...input); + + expect(logSpy).toHaveBeenCalledWith("Failed to handle pasted content as Safari inserted content"); + expect(output).toBe(false); + }); + + it("calls fetch when data types has text/html and data can parsed", () => { + const originalEvent = { + type: "paste", + clipboardData: { + files: [], + types: ["text/html"], + getData: jest.fn().mockReturnValue(``), + }, + } as unknown as ClipboardEvent; + const mockEventRelation = {} as unknown as IEventRelation; + const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; + handleClipboardEvent(...input); + + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(fetchSpy).toHaveBeenCalledWith("blob:"); + }); + + it("calls sendContentToRoom when parsing is successful", async () => { + fetchSpy.mockResolvedValueOnce({ + url: "test/file", + blob: () => { + return Promise.resolve({ type: "image/jpeg" } as Blob); + }, + } as Response); + + const originalEvent = { + type: "paste", + clipboardData: { + files: [], + types: ["text/html"], + getData: jest.fn().mockReturnValue(``), + }, + } as unknown as ClipboardEvent; + const mockEventRelation = {} as unknown as IEventRelation; + const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; + const output = handleClipboardEvent(...input); + + await waitFor(() => { + expect(sendContentToRoomSpy).toHaveBeenCalledTimes(1); + }); + expect(output).toBe(true); + }); +}); From c22510f6305eb916d2b0f5137e9ef87298afa822 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Tue, 6 Jun 2023 17:31:18 +0100 Subject: [PATCH 08/15] make behaviour mimic SendMessage --- .../hooks/useInputEventProcessor.ts | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index 3c7316f84bf..cfd8cadd62b 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -62,7 +62,8 @@ export function useInputEventProcessor( const isClipboardEvent = event instanceof ClipboardEvent; if (isClipboardEvent) { - return handleClipboardEvent(event, roomContext, mxClient, eventRelation); + const handled = handleClipboardEvent(event, roomContext, mxClient, eventRelation); + return handled ? null : event; } const isKeyboardEvent = event instanceof KeyboardEvent; @@ -235,36 +236,34 @@ function handleInputEvent(event: InputEvent, send: Send, isCtrlEnterToSend: bool } /** - * Takes a ClipboardEvent and handles image pasting. If the event is not a paste event, or - * pasting is not successful, returns the original event to allow it to pass through. - * Otherwise, returns null to indicate that the event has been handled. + * Takes a ClipboardEvent and handles image pasting. Returns a boolean to indicate if it has handled + * the event or not. * * @param clipboardEvent - event to process * @param roomContext - room in which the event occurs * @param mxClient - current matrix client * @param eventRelation - used to send the event to the correct timeline - * @returns - null if event is handled here, the `clipboardEvent` param if not + * @returns - boolean to show if the event was handled or not */ -function handleClipboardEvent( +export function handleClipboardEvent( clipboardEvent: ClipboardEvent, roomContext: IRoomState, mxClient: MatrixClient, eventRelation?: IEventRelation, -): ClipboardEvent | null { +): boolean { const { clipboardData: data } = clipboardEvent; const { room, timelineRenderingType, replyToEvent } = roomContext; - function errorHandler(error: unknown): ClipboardEvent { + function handleError(error: unknown): void { if (error instanceof Error) { console.log(error.message); - } else if (error instanceof String) { + } else if (typeof error === "string") { console.log(error); } - return clipboardEvent; } if (clipboardEvent.type !== "paste" || data === null || room === undefined) { - return clipboardEvent; + return false; } // Prioritize text on the clipboard over files if RTF is present as Office on macOS puts a bitmap @@ -274,8 +273,8 @@ function handleClipboardEvent( if (data.files.length && !data.types.includes("text/rtf")) { ContentMessages.sharedInstance() .sendContentListToRoom(Array.from(data.files), room.roomId, eventRelation, mxClient, timelineRenderingType) - .catch(errorHandler); - return null; + .catch(handleError); + return true; } // Safari `Insert from iPhone or iPad` @@ -290,7 +289,8 @@ function handleClipboardEvent( !imgDoc.querySelector("img")?.src.startsWith("blob:") || imgDoc.childNodes.length !== 1 ) { - errorHandler("Failed to handle pasted content as Safari inserted content"); + handleError("Failed to handle pasted content as Safari inserted content"); + return false; } const imgSrc = imgDoc!.querySelector("img")!.src; @@ -309,10 +309,10 @@ function handleClipboardEvent( mxClient, replyToEvent, ); - return null; - }, errorHandler); - }, errorHandler); + }, handleError); + }, handleError); + return true; } - return clipboardEvent; + return false; } From 2f70f31c591c85313cb131da8b21b0bfe9578acb Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 7 Jun 2023 09:11:28 +0100 Subject: [PATCH 09/15] add sad path tests --- .../hooks/useInputEventProcessor-test.tsx | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx b/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx index 84bda4d7edd..0959e0eebff 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx @@ -100,6 +100,16 @@ describe("handleClipboardEvent", () => { expect(output).toBe(false); }); + it("returns false if room clipboardData files and types are empty", () => { + const originalEvent = { + type: "paste", + clipboardData: { files: [], types: [] }, + } as unknown as ClipboardEvent; + const input = [originalEvent, mockRoomState, mockClient] as const; + const output = handleClipboardEvent(...input); + expect(output).toBe(false); + }); + it("handles event and calls sendContentListToRoom when data files are present", () => { const originalEvent = { type: "paste", @@ -192,6 +202,27 @@ describe("handleClipboardEvent", () => { expect(fetchSpy).toHaveBeenCalledWith("blob:"); }); + it("calls error handler when fetch fails", async () => { + const mockErrorMessage = "fetch failed"; + fetchSpy.mockRejectedValueOnce(mockErrorMessage); + const originalEvent = { + type: "paste", + clipboardData: { + files: [], + types: ["text/html"], + getData: jest.fn().mockReturnValue(``), + }, + } as unknown as ClipboardEvent; + const mockEventRelation = {} as unknown as IEventRelation; + const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; + const output = handleClipboardEvent(...input); + + await waitFor(() => { + expect(logSpy).toHaveBeenCalledWith(mockErrorMessage); + }); + expect(output).toBe(true); + }); + it("calls sendContentToRoom when parsing is successful", async () => { fetchSpy.mockResolvedValueOnce({ url: "test/file", @@ -217,4 +248,32 @@ describe("handleClipboardEvent", () => { }); expect(output).toBe(true); }); + + it("calls error handler when parsing is not successful", async () => { + fetchSpy.mockResolvedValueOnce({ + url: "test/file", + blob: () => { + return Promise.resolve({ type: "image/jpeg" } as Blob); + }, + } as Response); + const mockErrorMessage = "sendContentToRoom failed"; + sendContentToRoomSpy.mockRejectedValueOnce(mockErrorMessage); + + const originalEvent = { + type: "paste", + clipboardData: { + files: [], + types: ["text/html"], + getData: jest.fn().mockReturnValue(``), + }, + } as unknown as ClipboardEvent; + const mockEventRelation = {} as unknown as IEventRelation; + const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; + const output = handleClipboardEvent(...input); + + await waitFor(() => { + expect(logSpy).toHaveBeenCalledWith(mockErrorMessage); + }); + expect(output).toBe(true); + }); }); From 5ae4ff6e449ac905202d0bed2a31adc5ba228d77 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 7 Jun 2023 09:11:50 +0100 Subject: [PATCH 10/15] refactor to use catch throughout --- .../hooks/useInputEventProcessor.ts | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index cfd8cadd62b..e79cf05090b 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -292,25 +292,26 @@ export function handleClipboardEvent( handleError("Failed to handle pasted content as Safari inserted content"); return false; } - const imgSrc = imgDoc!.querySelector("img")!.src; - - fetch(imgSrc).then((response) => { - response.blob().then((imgBlob) => { - const type = imgBlob.type; - const safetype = getBlobSafeMimeType(type); - const ext = type.split("/")[1]; - const parts = response.url.split("/"); - const filename = parts[parts.length - 1]; - const file = new File([imgBlob], filename + "." + ext, { type: safetype }); - ContentMessages.sharedInstance().sendContentToRoom( - file, - room.roomId, - eventRelation, - mxClient, - replyToEvent, - ); - }, handleError); - }, handleError); + const imgSrc = imgDoc.querySelector("img")!.src; + + fetch(imgSrc) + .then((response) => { + response + .blob() + .then((imgBlob) => { + const type = imgBlob.type; + const safetype = getBlobSafeMimeType(type); + const ext = type.split("/")[1]; + const parts = response.url.split("/"); + const filename = parts[parts.length - 1]; + const file = new File([imgBlob], filename + "." + ext, { type: safetype }); + ContentMessages.sharedInstance() + .sendContentToRoom(file, room.roomId, eventRelation, mxClient, replyToEvent) + .catch(handleError); + }) + .catch(handleError); + }) + .catch(handleError); return true; } From 14a62c790daf101613211748fde9af9fdfefc99a Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 7 Jun 2023 09:49:34 +0100 Subject: [PATCH 11/15] update comments --- .../rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index e79cf05090b..1b7fe315b70 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -242,7 +242,7 @@ function handleInputEvent(event: InputEvent, send: Send, isCtrlEnterToSend: bool * @param clipboardEvent - event to process * @param roomContext - room in which the event occurs * @param mxClient - current matrix client - * @param eventRelation - used to send the event to the correct timeline + * @param eventRelation - used to send the event to the correct place eg timeline vs thread * @returns - boolean to show if the event was handled or not */ export function handleClipboardEvent( @@ -251,6 +251,7 @@ export function handleClipboardEvent( mxClient: MatrixClient, eventRelation?: IEventRelation, ): boolean { + // Logic in this function follows that of `SendMessageComposer.onPaste` const { clipboardData: data } = clipboardEvent; const { room, timelineRenderingType, replyToEvent } = roomContext; From e342e780836dbbcf8606d7a8539a1fb79f104242 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Wed, 7 Jun 2023 10:06:28 +0100 Subject: [PATCH 12/15] tidy up tests --- .../hooks/useInputEventProcessor-test.tsx | 157 +++++++----------- 1 file changed, 62 insertions(+), 95 deletions(-) diff --git a/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx b/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx index 0959e0eebff..211f5d10bf2 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx @@ -13,63 +13,29 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -import { IEventRelation } from "matrix-js-sdk/src/matrix"; +import { IEventRelation, MatrixEvent } from "matrix-js-sdk/src/matrix"; import { waitFor } from "@testing-library/react"; import { handleClipboardEvent } from "../../../../../../src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor"; import { TimelineRenderingType } from "../../../../../../src/contexts/RoomContext"; -import { Layout } from "../../../../../../src/settings/enums/Layout"; import { mkStubRoom, stubClient } from "../../../../../test-utils"; import ContentMessages from "../../../../../../src/ContentMessages"; +import { IRoomState } from "../../../../../../src/components/structures/RoomView"; -describe("handleClipboardEvent", () => { - const mockClient = stubClient(); - const mockRoom = mkStubRoom("mock room", "mock room", mockClient); - const mockRoomState = { - room: mockRoom, - roomLoading: true, - peekLoading: false, - shouldPeek: true, - membersLoaded: false, - numUnreadMessages: 0, - canPeek: false, - showApps: false, - isPeeking: false, - showRightPanel: true, - joining: false, - showTopUnreadMessagesBar: false, - statusBarVisible: false, - canReact: false, - canSelfRedact: false, - canSendMessages: false, - resizing: false, - layout: Layout.Group, - lowBandwidth: false, - alwaysShowTimestamps: false, - showTwelveHourTimestamps: false, - readMarkerInViewThresholdMs: 3000, - readMarkerOutOfViewThresholdMs: 30000, - showHiddenEvents: false, - showReadReceipts: true, - showRedactions: true, - showJoinLeaves: true, - showAvatarChanges: true, - showDisplaynameChanges: true, - matrixClientIsReady: false, - showUrlPreview: false, - timelineRenderingType: TimelineRenderingType.Room, - threadId: undefined, - liveTimeline: undefined, - narrow: false, - activeCall: null, - msc3946ProcessDynamicPredecessor: false, - }; - - const sendContentListToRoomSpy = jest.spyOn(ContentMessages.sharedInstance(), "sendContentListToRoom"); - const sendContentToRoomSpy = jest.spyOn(ContentMessages.sharedInstance(), "sendContentToRoom"); - const fetchSpy = jest.spyOn(window, "fetch"); - const logSpy = jest.spyOn(console, "log").mockImplementation(() => {}); +const mockClient = stubClient(); +const mockRoom = mkStubRoom("mock room", "mock room", mockClient); +const mockRoomState = { + room: mockRoom, + timelineRenderingType: TimelineRenderingType.Room, + replyToEvent: {} as unknown as MatrixEvent, +} as unknown as IRoomState; + +const sendContentListToRoomSpy = jest.spyOn(ContentMessages.sharedInstance(), "sendContentListToRoom"); +const sendContentToRoomSpy = jest.spyOn(ContentMessages.sharedInstance(), "sendContentToRoom"); +const fetchSpy = jest.spyOn(window, "fetch"); +const logSpy = jest.spyOn(console, "log").mockImplementation(() => {}); +describe("handleClipboardEvent", () => { beforeEach(() => { jest.clearAllMocks(); }); @@ -78,45 +44,47 @@ describe("handleClipboardEvent", () => { jest.restoreAllMocks(); }); + function createMockClipboardEvent(props: any): ClipboardEvent { + return { ...props } as ClipboardEvent; + } + it("returns false if it is not a paste event", () => { - const originalEvent = { type: "copy" } as ClipboardEvent; - const input = [originalEvent, mockRoomState, mockClient] as const; - const output = handleClipboardEvent(...input); + const originalEvent = createMockClipboardEvent({ type: "copy" }); + const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient); + expect(output).toBe(false); }); it("returns false if clipboard data is null", () => { - const originalEvent = { type: "paste", clipboardData: null } as ClipboardEvent; - const input = [originalEvent, mockRoomState, mockClient] as const; - const output = handleClipboardEvent(...input); + const originalEvent = createMockClipboardEvent({ type: "paste", clipboardData: null }); + const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient); + expect(output).toBe(false); }); it("returns false if room is undefined", () => { - const originalEvent = { type: "paste" } as ClipboardEvent; + const originalEvent = createMockClipboardEvent({ type: "paste" }); const { room, ...roomStateWithoutRoom } = mockRoomState; - const input = [originalEvent, roomStateWithoutRoom, mockClient] as const; - const output = handleClipboardEvent(...input); + const output = handleClipboardEvent(originalEvent, roomStateWithoutRoom, mockClient); + expect(output).toBe(false); }); it("returns false if room clipboardData files and types are empty", () => { - const originalEvent = { + const originalEvent = createMockClipboardEvent({ type: "paste", clipboardData: { files: [], types: [] }, - } as unknown as ClipboardEvent; - const input = [originalEvent, mockRoomState, mockClient] as const; - const output = handleClipboardEvent(...input); + }); + const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient); expect(output).toBe(false); }); it("handles event and calls sendContentListToRoom when data files are present", () => { - const originalEvent = { + const originalEvent = createMockClipboardEvent({ type: "paste", clipboardData: { files: ["something here"], types: [] }, - } as unknown as ClipboardEvent; - const input = [originalEvent, mockRoomState, mockClient] as const; - const output = handleClipboardEvent(...input); + }); + const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient); expect(sendContentListToRoomSpy).toHaveBeenCalledTimes(1); expect(sendContentListToRoomSpy).toHaveBeenCalledWith( @@ -130,13 +98,12 @@ describe("handleClipboardEvent", () => { }); it("calls sendContentListToRoom with eventRelation when present", () => { - const originalEvent = { + const originalEvent = createMockClipboardEvent({ type: "paste", clipboardData: { files: ["something here"], types: [] }, - } as unknown as ClipboardEvent; + }); const mockEventRelation = {} as unknown as IEventRelation; - const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; - const output = handleClipboardEvent(...input); + const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); expect(sendContentListToRoomSpy).toHaveBeenCalledTimes(1); expect(sendContentListToRoomSpy).toHaveBeenCalledWith( @@ -153,13 +120,12 @@ describe("handleClipboardEvent", () => { const mockErrorMessage = "something went wrong"; sendContentListToRoomSpy.mockRejectedValueOnce(new Error(mockErrorMessage)); - const originalEvent = { + const originalEvent = createMockClipboardEvent({ type: "paste", clipboardData: { files: ["something here"], types: [] }, - } as unknown as ClipboardEvent; + }); const mockEventRelation = {} as unknown as IEventRelation; - const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; - const output = handleClipboardEvent(...input); + const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); expect(sendContentListToRoomSpy).toHaveBeenCalledTimes(1); await waitFor(() => { @@ -169,34 +135,32 @@ describe("handleClipboardEvent", () => { }); it("calls the error handler when data types has text/html but data can not be parsed", () => { - const originalEvent = { + const originalEvent = createMockClipboardEvent({ type: "paste", clipboardData: { files: [], types: ["text/html"], getData: jest.fn().mockReturnValue("
invalid html"), }, - } as unknown as ClipboardEvent; + }); const mockEventRelation = {} as unknown as IEventRelation; - const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; - const output = handleClipboardEvent(...input); + const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); expect(logSpy).toHaveBeenCalledWith("Failed to handle pasted content as Safari inserted content"); expect(output).toBe(false); }); it("calls fetch when data types has text/html and data can parsed", () => { - const originalEvent = { + const originalEvent = createMockClipboardEvent({ type: "paste", clipboardData: { files: [], types: ["text/html"], getData: jest.fn().mockReturnValue(``), }, - } as unknown as ClipboardEvent; + }); const mockEventRelation = {} as unknown as IEventRelation; - const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; - handleClipboardEvent(...input); + handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); expect(fetchSpy).toHaveBeenCalledTimes(1); expect(fetchSpy).toHaveBeenCalledWith("blob:"); @@ -205,17 +169,16 @@ describe("handleClipboardEvent", () => { it("calls error handler when fetch fails", async () => { const mockErrorMessage = "fetch failed"; fetchSpy.mockRejectedValueOnce(mockErrorMessage); - const originalEvent = { + const originalEvent = createMockClipboardEvent({ type: "paste", clipboardData: { files: [], types: ["text/html"], getData: jest.fn().mockReturnValue(``), }, - } as unknown as ClipboardEvent; + }); const mockEventRelation = {} as unknown as IEventRelation; - const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; - const output = handleClipboardEvent(...input); + const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); await waitFor(() => { expect(logSpy).toHaveBeenCalledWith(mockErrorMessage); @@ -231,20 +194,25 @@ describe("handleClipboardEvent", () => { }, } as Response); - const originalEvent = { + const originalEvent = createMockClipboardEvent({ type: "paste", clipboardData: { files: [], types: ["text/html"], getData: jest.fn().mockReturnValue(``), }, - } as unknown as ClipboardEvent; + }); const mockEventRelation = {} as unknown as IEventRelation; - const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; - const output = handleClipboardEvent(...input); + const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); await waitFor(() => { - expect(sendContentToRoomSpy).toHaveBeenCalledTimes(1); + expect(sendContentToRoomSpy).toHaveBeenCalledWith( + expect.any(File), + mockRoom.roomId, + mockEventRelation, + mockClient, + mockRoomState.replyToEvent, + ); }); expect(output).toBe(true); }); @@ -259,17 +227,16 @@ describe("handleClipboardEvent", () => { const mockErrorMessage = "sendContentToRoom failed"; sendContentToRoomSpy.mockRejectedValueOnce(mockErrorMessage); - const originalEvent = { + const originalEvent = createMockClipboardEvent({ type: "paste", clipboardData: { files: [], types: ["text/html"], getData: jest.fn().mockReturnValue(``), }, - } as unknown as ClipboardEvent; + }); const mockEventRelation = {} as unknown as IEventRelation; - const input = [originalEvent, mockRoomState, mockClient, mockEventRelation] as const; - const output = handleClipboardEvent(...input); + const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); await waitFor(() => { expect(logSpy).toHaveBeenCalledWith(mockErrorMessage); From dafe5858385ce990cd584c8e1e4363a38d82bd49 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Thu, 8 Jun 2023 13:30:56 +0100 Subject: [PATCH 13/15] add special case and change function signature --- .../hooks/useInputEventProcessor.ts | 26 ++++--- .../hooks/useInputEventProcessor-test.tsx | 67 +++++++++++++++---- 2 files changed, 72 insertions(+), 21 deletions(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index 1b7fe315b70..c289f8c039c 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -36,6 +36,7 @@ import Autocomplete from "../../Autocomplete"; import { handleEventWithAutocomplete } from "./utils"; import ContentMessages from "../../../../../ContentMessages"; import { getBlobSafeMimeType } from "../../../../../utils/blobs"; +import { isNotNull } from "../../../../../Typeguards"; export function useInputEventProcessor( onSend: () => void, @@ -60,9 +61,17 @@ export function useInputEventProcessor( onSend(); }; + // this is required to handle edge case image pasting in Safari, see + // https://github.com/vector-im/element-web/issues/25327 + const isInputEventForClipboard = + event instanceof InputEvent && event.inputType === "insertFromPaste" && isNotNull(event.dataTransfer); const isClipboardEvent = event instanceof ClipboardEvent; - if (isClipboardEvent) { - const handled = handleClipboardEvent(event, roomContext, mxClient, eventRelation); + + const shouldHandleAsClipboardEvent = isClipboardEvent || isInputEventForClipboard; + + if (shouldHandleAsClipboardEvent) { + const data = isClipboardEvent ? event.clipboardData : event.dataTransfer; + const handled = handleClipboardEvent(event, data, roomContext, mxClient, eventRelation); return handled ? null : event; } @@ -236,23 +245,24 @@ function handleInputEvent(event: InputEvent, send: Send, isCtrlEnterToSend: bool } /** - * Takes a ClipboardEvent and handles image pasting. Returns a boolean to indicate if it has handled - * the event or not. + * Takes an event and handles image pasting. Returns a boolean to indicate if it has handled + * the event or not. Must accept either clipboard or input events in order to prevent issue: + * https://github.com/vector-im/element-web/issues/25327 * - * @param clipboardEvent - event to process + * @param event - event to process * @param roomContext - room in which the event occurs * @param mxClient - current matrix client * @param eventRelation - used to send the event to the correct place eg timeline vs thread * @returns - boolean to show if the event was handled or not */ export function handleClipboardEvent( - clipboardEvent: ClipboardEvent, + event: ClipboardEvent | InputEvent, + data: DataTransfer | null, roomContext: IRoomState, mxClient: MatrixClient, eventRelation?: IEventRelation, ): boolean { // Logic in this function follows that of `SendMessageComposer.onPaste` - const { clipboardData: data } = clipboardEvent; const { room, timelineRenderingType, replyToEvent } = roomContext; function handleError(error: unknown): void { @@ -263,7 +273,7 @@ export function handleClipboardEvent( } } - if (clipboardEvent.type !== "paste" || data === null || room === undefined) { + if (event.type !== "paste" || data === null || room === undefined) { return false; } diff --git a/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx b/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx index 211f5d10bf2..8d6f9d19cc4 100644 --- a/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor-test.tsx @@ -45,19 +45,19 @@ describe("handleClipboardEvent", () => { }); function createMockClipboardEvent(props: any): ClipboardEvent { - return { ...props } as ClipboardEvent; + return { clipboardData: { files: [], types: [] }, ...props } as ClipboardEvent; } it("returns false if it is not a paste event", () => { const originalEvent = createMockClipboardEvent({ type: "copy" }); - const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient); + const output = handleClipboardEvent(originalEvent, originalEvent.clipboardData, mockRoomState, mockClient); expect(output).toBe(false); }); it("returns false if clipboard data is null", () => { const originalEvent = createMockClipboardEvent({ type: "paste", clipboardData: null }); - const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient); + const output = handleClipboardEvent(originalEvent, originalEvent.clipboardData, mockRoomState, mockClient); expect(output).toBe(false); }); @@ -65,7 +65,12 @@ describe("handleClipboardEvent", () => { it("returns false if room is undefined", () => { const originalEvent = createMockClipboardEvent({ type: "paste" }); const { room, ...roomStateWithoutRoom } = mockRoomState; - const output = handleClipboardEvent(originalEvent, roomStateWithoutRoom, mockClient); + const output = handleClipboardEvent( + originalEvent, + originalEvent.clipboardData, + roomStateWithoutRoom, + mockClient, + ); expect(output).toBe(false); }); @@ -75,7 +80,7 @@ describe("handleClipboardEvent", () => { type: "paste", clipboardData: { files: [], types: [] }, }); - const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient); + const output = handleClipboardEvent(originalEvent, originalEvent.clipboardData, mockRoomState, mockClient); expect(output).toBe(false); }); @@ -84,7 +89,7 @@ describe("handleClipboardEvent", () => { type: "paste", clipboardData: { files: ["something here"], types: [] }, }); - const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient); + const output = handleClipboardEvent(originalEvent, originalEvent.clipboardData, mockRoomState, mockClient); expect(sendContentListToRoomSpy).toHaveBeenCalledTimes(1); expect(sendContentListToRoomSpy).toHaveBeenCalledWith( @@ -103,7 +108,13 @@ describe("handleClipboardEvent", () => { clipboardData: { files: ["something here"], types: [] }, }); const mockEventRelation = {} as unknown as IEventRelation; - const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); + const output = handleClipboardEvent( + originalEvent, + originalEvent.clipboardData, + mockRoomState, + mockClient, + mockEventRelation, + ); expect(sendContentListToRoomSpy).toHaveBeenCalledTimes(1); expect(sendContentListToRoomSpy).toHaveBeenCalledWith( @@ -125,7 +136,13 @@ describe("handleClipboardEvent", () => { clipboardData: { files: ["something here"], types: [] }, }); const mockEventRelation = {} as unknown as IEventRelation; - const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); + const output = handleClipboardEvent( + originalEvent, + originalEvent.clipboardData, + mockRoomState, + mockClient, + mockEventRelation, + ); expect(sendContentListToRoomSpy).toHaveBeenCalledTimes(1); await waitFor(() => { @@ -144,7 +161,13 @@ describe("handleClipboardEvent", () => { }, }); const mockEventRelation = {} as unknown as IEventRelation; - const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); + const output = handleClipboardEvent( + originalEvent, + originalEvent.clipboardData, + mockRoomState, + mockClient, + mockEventRelation, + ); expect(logSpy).toHaveBeenCalledWith("Failed to handle pasted content as Safari inserted content"); expect(output).toBe(false); @@ -160,7 +183,7 @@ describe("handleClipboardEvent", () => { }, }); const mockEventRelation = {} as unknown as IEventRelation; - handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); + handleClipboardEvent(originalEvent, originalEvent.clipboardData, mockRoomState, mockClient, mockEventRelation); expect(fetchSpy).toHaveBeenCalledTimes(1); expect(fetchSpy).toHaveBeenCalledWith("blob:"); @@ -178,7 +201,13 @@ describe("handleClipboardEvent", () => { }, }); const mockEventRelation = {} as unknown as IEventRelation; - const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); + const output = handleClipboardEvent( + originalEvent, + originalEvent.clipboardData, + mockRoomState, + mockClient, + mockEventRelation, + ); await waitFor(() => { expect(logSpy).toHaveBeenCalledWith(mockErrorMessage); @@ -203,7 +232,13 @@ describe("handleClipboardEvent", () => { }, }); const mockEventRelation = {} as unknown as IEventRelation; - const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); + const output = handleClipboardEvent( + originalEvent, + originalEvent.clipboardData, + mockRoomState, + mockClient, + mockEventRelation, + ); await waitFor(() => { expect(sendContentToRoomSpy).toHaveBeenCalledWith( @@ -236,7 +271,13 @@ describe("handleClipboardEvent", () => { }, }); const mockEventRelation = {} as unknown as IEventRelation; - const output = handleClipboardEvent(originalEvent, mockRoomState, mockClient, mockEventRelation); + const output = handleClipboardEvent( + originalEvent, + originalEvent.clipboardData, + mockRoomState, + mockClient, + mockEventRelation, + ); await waitFor(() => { expect(logSpy).toHaveBeenCalledWith(mockErrorMessage); From 77ccbf83baa316de329766076fcd233b3cba131f Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 9 Jun 2023 09:27:23 +0100 Subject: [PATCH 14/15] add comment --- .../rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts index c289f8c039c..27f40880140 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useInputEventProcessor.ts @@ -62,7 +62,8 @@ export function useInputEventProcessor( }; // this is required to handle edge case image pasting in Safari, see - // https://github.com/vector-im/element-web/issues/25327 + // https://github.com/vector-im/element-web/issues/25327 and it is caught by the + // `beforeinput` listener attached to the composer const isInputEventForClipboard = event instanceof InputEvent && event.inputType === "insertFromPaste" && isNotNull(event.dataTransfer); const isClipboardEvent = event instanceof ClipboardEvent; From 3a433fe0cd2512fce697b231a3ae0e3b16e38b26 Mon Sep 17 00:00:00 2001 From: Alun Turner Date: Fri, 9 Jun 2023 09:39:01 +0100 Subject: [PATCH 15/15] bump rte to 2.2.2 --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 30dba62b743..9cc82f94bdc 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "dependencies": { "@babel/runtime": "^7.12.5", "@matrix-org/analytics-events": "^0.5.0", - "@matrix-org/matrix-wysiwyg": "^2.0.0", + "@matrix-org/matrix-wysiwyg": "^2.2.2", "@matrix-org/react-sdk-module-api": "^0.0.5", "@sentry/browser": "^7.0.0", "@sentry/tracing": "^7.0.0", diff --git a/yarn.lock b/yarn.lock index 80345b92458..82baee84e67 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1598,10 +1598,10 @@ resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-js/-/matrix-sdk-crypto-js-0.1.0-alpha.8.tgz#18dd8e7fb56602d2999d8a502b49e902a2bb3782" integrity sha512-hdmbbGXKrN6JNo3wdBaR5Zs3lXlzllT3U43ViNTlabB3nKkOZQnEAN/Isv+4EQSgz1+8897veI9Q8sqlQX22oA== -"@matrix-org/matrix-wysiwyg@^2.0.0": - version "2.2.1" - resolved "https://registry.yarnpkg.com/@matrix-org/matrix-wysiwyg/-/matrix-wysiwyg-2.2.1.tgz#076b409c0ffe655938d663863b1ee546a7101da6" - integrity sha512-QF4dJsyqBMxZx+GhSdSiRSDIuwE5dxd7vffQ5i6hf67bd0EbVvtf4PzWmNopGHA+ckjMJIc5X1EPT+6DG/wM6Q== +"@matrix-org/matrix-wysiwyg@^2.2.2": + version "2.2.2" + resolved "https://registry.yarnpkg.com/@matrix-org/matrix-wysiwyg/-/matrix-wysiwyg-2.2.2.tgz#911d0a9858a5a4b620f93777085daac8eff6a220" + integrity sha512-FprkgKiqEHoFUfaamKwTGBENqDxbORFgoPjiE1b9yPS3hgRswobVKRl4qrXgVgFj4qQ7gWeTqogiyrHXkm1myw== "@matrix-org/olm@https://gitlab.matrix.org/api/v4/projects/27/packages/npm/@matrix-org/olm/-/@matrix-org/olm-3.2.14.tgz": version "3.2.14"