Resolve issues

src/Notifier.js

@@ -97,20 +97,24 @@ const Notifier = {
     },
 
     getSoundForRoom: async function(roomId) {
-        // We do no caching here because the SDK caches the event content
+        // We do no caching here because the SDK caches setting
         // and the browser will cache the sound.
         let content = SettingsStore.getValue("notificationSound", roomId);
         if (!content) {
-            content = SettingsStore.getValue("notificationSound");
-            if (!content) {
-                return null;
-            }
+            return null;
         }
 
         if (!content.url) {
             console.warn(`${roomId} has custom notification sound event, but no url key`);
             return null;
         }
+        
+        if (!content.url.startsWith("mxc://")) {
+            console.warn(`${roomId} has custom notification sound event, but url is not a mxc url`);
+            return null;
+        }
+
+        // Ideally in here we could use MSC1310 to detect the type of file, and reject it.
 
         return {
             url: MatrixClientPeg.get().mxcUrlToHttp(content.url),
@@ -123,7 +127,7 @@ const Notifier = {
     _playAudioNotification: async function(ev, room) {
         const sound = SettingsStore.isFeatureEnabled("feature_notification_sounds") ? await this.getSoundForRoom(room.roomId) : null;
         console.log(`Got sound ${sound && sound.name || "default"} for ${room.roomId}`);
-        // XXX: How do we ensure this is a sound file and not going to be exploited?
+
         try {
             const selector = document.querySelector(sound ? `audio[src='${sound.url}']` : "#messageAudio");
             let audioElement = selector;

src/components/views/settings/tabs/room/NotificationSettingsTab.js

@@ -20,12 +20,12 @@ import {_t} from "../../../../../languageHandler";
 import MatrixClientPeg from "../../../../../MatrixClientPeg";
 import AccessibleButton from "../../../elements/AccessibleButton";
 import Notifier from "../../../../../Notifier";
-import SettingsStore from '../../../../../settings/SettingsStore';
+import SettingsStore from  '../../../../../settings/SettingsStore';
+import { SettingLevel } from '../../../../../settings/SettingsStore';
 
 export default class NotificationsSettingsTab extends React.Component {
     static propTypes = {
         roomId: PropTypes.string.isRequired,
-        closeSettingsFn: PropTypes.func.isRequired,
     };
 
     constructor() {
@@ -46,7 +46,7 @@ export default class NotificationsSettingsTab extends React.Component {
         });
     }
 
-    _onSoundUploadChanged(e) {
+    async _onSoundUploadChanged(e) {
         if (!e.target.files || !e.target.files.length) {
             this.setState({
                 uploadedFile: null,
@@ -58,11 +58,18 @@ export default class NotificationsSettingsTab extends React.Component {
         this.setState({
             uploadedFile: file,
         });
+
+        try {
+            await this._saveSound();
+        } catch (ex) {
+            console.error(
+                `Unable to save notification sound for ${this.props.roomId}`,
+                ex,
+            );
+        }
     }
 
-    async _saveSound(e) {
-        e.stopPropagation();
-        e.preventDefault();
+    async _saveSound() {
         if (!this.state.uploadedFile) {
             return;
         }
@@ -82,7 +89,8 @@ export default class NotificationsSettingsTab extends React.Component {
         await SettingsStore.setValue(
             "notificationSound",
             this.props.roomId,
-            "room-account",
+            SettingsStore.
+            SettingLevel.ROOM_ACCOUNT,
             {
                 name: this.state.uploadedFile.name,
                 type: type,
@@ -101,7 +109,12 @@ export default class NotificationsSettingsTab extends React.Component {
     _clearSound(e) {
         e.stopPropagation();
         e.preventDefault();
-        SettingsStore.setValue("notificationSound", this.props.roomId, "room-account", null);
+        SettingsStore.setValue(
+            "notificationSound",
+            this.props.roomId,
+            SettingLevel.ROOM_ACCOUNT,
+            null,
+        );
 
         this.setState({
             currentSound: "default",
@@ -119,11 +132,8 @@ export default class NotificationsSettingsTab extends React.Component {
                     </div>
                     <div>
                         <h3>{_t("Set a new custom sound")}</h3>
-                        <form onSubmit={this._saveSound.bind(this)} autoComplete={false} noValidate={true}>
+                        <form autoComplete={false} noValidate={true}>
                             <input type="file" onChange={this._onSoundUploadChanged.bind(this)} accept="audio/*" />
-                            <AccessibleButton onClick={this._saveSound.bind(this)} kind="primary" disabled={!this.state.uploadedFile}>
-                                {_t("Save")}
-                            </AccessibleButton>
                         </form>
                         <AccessibleButton onClick={this._clearSound.bind(this)} kind="primary">
                                 {_t("Reset to default sound")}

src/i18n/strings/en_EN.json

@@ -297,6 +297,7 @@
     "Show recent room avatars above the room list": "Show recent room avatars above the room list",
     "Group & filter rooms by custom tags (refresh to apply changes)": "Group & filter rooms by custom tags (refresh to apply changes)",
     "Render simple counters in room header": "Render simple counters in room header",
+    "Custom Notification Sounds": "Custom Notification Sounds",
     "React to messages with emoji": "React to messages with emoji",
     "Enable Emoji suggestions while typing": "Enable Emoji suggestions while typing",
     "Use compact timeline layout": "Use compact timeline layout",
@@ -613,6 +614,9 @@
     "Room Addresses": "Room Addresses",
     "Publish this room to the public in %(domain)s's room directory?": "Publish this room to the public in %(domain)s's room directory?",
     "URL Previews": "URL Previews",
+    "Sounds": "Sounds",
+    "Set a new custom sound": "Set a new custom sound",
+    "Reset to default sound": "Reset to default sound",
     "Change room avatar": "Change room avatar",
     "Change room name": "Change room name",
     "Change main address for the room": "Change main address for the room",
@@ -1619,9 +1623,5 @@
     "If you didn't remove the recovery method, an attacker may be trying to access your account. Change your account password and set a new recovery method immediately in Settings.": "If you didn't remove the recovery method, an attacker may be trying to access your account. Change your account password and set a new recovery method immediately in Settings.",
     "Failed to set direct chat tag": "Failed to set direct chat tag",
     "Failed to remove tag %(tagName)s from room": "Failed to remove tag %(tagName)s from room",
-    "Failed to add tag %(tagName)s to room": "Failed to add tag %(tagName)s to room",
-    "Sounds": "Sounds",
-    "Custom Notification Sounds": "Notification sound",
-    "Set a new custom sound": "Set a new custom sound",
-    "Reset to default sound": "Reset to default sound"
+    "Failed to add tag %(tagName)s to room": "Failed to add tag %(tagName)s to room"
 }

src/settings/Settings.js

@@ -122,6 +122,7 @@ export const SETTINGS = {
     "feature_notification_sounds": {
         isFeature: true,
         displayName: _td("Custom Notification Sounds"),
+    },
     "feature_reactions": {
         isFeature: true,
         displayName: _td("React to messages with emoji"),