From 77764d80bc20f95566e79071c9131bd5d9269d7d Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 7 Nov 2022 11:56:43 +0000 Subject: [PATCH] Fix regressions around media uploads failing and causing soft crashes (#9549) --- src/Notifier.ts | 4 +- .../views/settings/ChangeAvatar.tsx | 209 ------------------ .../tabs/room/NotificationSettingsTab.tsx | 2 +- .../views/spaces/SpaceSettingsGeneralTab.tsx | 9 +- src/i18n/strings/en_EN.json | 2 - test/Notifier-test.ts | 9 + 6 files changed, 17 insertions(+), 218 deletions(-) delete mode 100644 src/components/views/settings/ChangeAvatar.tsx diff --git a/src/Notifier.ts b/src/Notifier.ts index b75b821ae8d..2f925699284 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -161,8 +161,8 @@ export const Notifier = { return null; } - if (!content.url) { - logger.warn(`${roomId} has custom notification sound event, but no url key`); + if (typeof content.url !== "string") { + logger.warn(`${roomId} has custom notification sound event, but no url string`); return null; } diff --git a/src/components/views/settings/ChangeAvatar.tsx b/src/components/views/settings/ChangeAvatar.tsx deleted file mode 100644 index 680291db4ce..00000000000 --- a/src/components/views/settings/ChangeAvatar.tsx +++ /dev/null @@ -1,209 +0,0 @@ -/* -Copyright 2015-2021 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 React from 'react'; -import { MatrixEvent } from 'matrix-js-sdk/src/models/event'; -import { Room } from 'matrix-js-sdk/src/models/room'; -import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; -import { EventType } from "matrix-js-sdk/src/@types/event"; - -import { MatrixClientPeg } from "../../../MatrixClientPeg"; -import { _t } from '../../../languageHandler'; -import Spinner from '../elements/Spinner'; -import { mediaFromMxc } from "../../../customisations/Media"; -import RoomAvatar from '../avatars/RoomAvatar'; -import BaseAvatar from '../avatars/BaseAvatar'; -import { chromeFileInputFix } from "../../../utils/BrowserWorkarounds"; - -interface IProps { - initialAvatarUrl?: string; - room?: Room; - // if false, you need to call changeAvatar.onFileSelected yourself. - showUploadSection?: boolean; - width?: number; - height?: number; - className?: string; -} - -interface IState { - avatarUrl?: string; - errorText?: string; - phase?: Phases; -} - -enum Phases { - Display = "display", - Uploading = "uploading", - Error = "error", -} - -export default class ChangeAvatar extends React.Component { - public static defaultProps = { - showUploadSection: true, - className: "", - width: 80, - height: 80, - }; - - private avatarSet = false; - - constructor(props: IProps) { - super(props); - - this.state = { - avatarUrl: this.props.initialAvatarUrl, - phase: Phases.Display, - }; - } - - public componentDidMount(): void { - MatrixClientPeg.get().on(RoomStateEvent.Events, this.onRoomStateEvents); - } - - // TODO: [REACT-WARNING] Replace with appropriate lifecycle event - // eslint-disable-next-line - public UNSAFE_componentWillReceiveProps(newProps: IProps): void { - if (this.avatarSet) { - // don't clobber what the user has just set - return; - } - this.setState({ - avatarUrl: newProps.initialAvatarUrl, - }); - } - - public componentWillUnmount(): void { - if (MatrixClientPeg.get()) { - MatrixClientPeg.get().removeListener(RoomStateEvent.Events, this.onRoomStateEvents); - } - } - - private onRoomStateEvents = (ev: MatrixEvent) => { - if (!this.props.room) { - return; - } - - if (ev.getRoomId() !== this.props.room.roomId || - ev.getType() !== EventType.RoomAvatar || - ev.getSender() !== MatrixClientPeg.get().getUserId() - ) { - return; - } - - if (!ev.getContent().url) { - this.avatarSet = false; - this.setState({}); // force update - } - }; - - private setAvatarFromFile(file: File): Promise<{}> { - let newUrl = null; - - this.setState({ - phase: Phases.Uploading, - }); - const httpPromise = MatrixClientPeg.get().uploadContent(file).then(({ content_uri: url }) => { - newUrl = url; - if (this.props.room) { - return MatrixClientPeg.get().sendStateEvent( - this.props.room.roomId, - 'm.room.avatar', - { url }, - '', - ); - } else { - return MatrixClientPeg.get().setAvatarUrl(url); - } - }); - - httpPromise.then(() => { - this.setState({ - phase: Phases.Display, - avatarUrl: mediaFromMxc(newUrl).srcHttp, - }); - }, () => { - this.setState({ - phase: Phases.Error, - }); - this.onError(); - }); - - return httpPromise; - } - - private onFileSelected = (ev: React.ChangeEvent) => { - this.avatarSet = true; - return this.setAvatarFromFile(ev.target.files[0]); - }; - - private onError = (): void => { - this.setState({ - errorText: _t("Failed to upload profile picture!"), - }); - }; - - public render(): JSX.Element { - let avatarImg; - // Having just set an avatar we just display that since it will take a little - // time to propagate through to the RoomAvatar. - if (this.props.room && !this.avatarSet) { - avatarImg = ; - } else { - // XXX: FIXME: once we track in the JS what our own displayname is(!) then use it here rather than ? - avatarImg = ; - } - - let uploadSection; - if (this.props.showUploadSection) { - uploadSection = ( -
- { _t("Upload new:") } - - { this.state.errorText } -
- ); - } - - switch (this.state.phase) { - case Phases.Display: - case Phases.Error: - return ( -
-
- { avatarImg } -
- { uploadSection } -
- ); - case Phases.Uploading: - return ( - - ); - } - } -} diff --git a/src/components/views/settings/tabs/room/NotificationSettingsTab.tsx b/src/components/views/settings/tabs/room/NotificationSettingsTab.tsx index 125b60e0bd7..2ac282654a9 100644 --- a/src/components/views/settings/tabs/room/NotificationSettingsTab.tsx +++ b/src/components/views/settings/tabs/room/NotificationSettingsTab.tsx @@ -119,7 +119,7 @@ export default class NotificationsSettingsTab extends React.Component { setBusy(true); - const promises = []; + const promises: Promise[] = []; if (avatarChanged) { if (newAvatar) { - promises.push(cli.sendStateEvent(space.roomId, EventType.RoomAvatar, { - url: await cli.uploadContent(newAvatar), - }, "")); + promises.push((async () => { + const { content_uri: url } = await cli.uploadContent(newAvatar); + await cli.sendStateEvent(space.roomId, EventType.RoomAvatar, { url }, ""); + })()); } else { promises.push(cli.sendStateEvent(space.roomId, EventType.RoomAvatar, {}, "")); } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 2dd52d7b031..4503ccfa823 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1283,8 +1283,6 @@ "This bridge is managed by .": "This bridge is managed by .", "Workspace: ": "Workspace: ", "Channel: ": "Channel: ", - "Failed to upload profile picture!": "Failed to upload profile picture!", - "Upload new:": "Upload new:", "No display name": "No display name", "Warning!": "Warning!", "Changing your password on this homeserver will cause all of your other devices to be signed out. This will delete the message encryption keys stored on them, and may make encrypted chat history unreadable.": "Changing your password on this homeserver will cause all of your other devices to be signed out. This will delete the message encryption keys stored on them, and may make encrypted chat history unreadable.", diff --git a/test/Notifier-test.ts b/test/Notifier-test.ts index e4d4580cb75..498151fa1f0 100644 --- a/test/Notifier-test.ts +++ b/test/Notifier-test.ts @@ -232,6 +232,15 @@ describe("Notifier", () => { }); }); + describe("getSoundForRoom", () => { + it("should not explode if given invalid url", () => { + jest.spyOn(SettingsStore, "getValue").mockImplementation((name: string) => { + return { url: { content_uri: "foobar" } }; + }); + expect(Notifier.getSoundForRoom("!roomId:server")).toBeNull(); + }); + }); + describe("_playAudioNotification", () => { it.each([ { event: { is_silenced: true }, count: 0 },