From 878663ca077041db13b9983c02f074f725e9ccdd Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Mon, 30 Aug 2021 11:01:56 -0700 Subject: [PATCH 1/8] Start work on handling invalid URLs --- src/RootViewModel.js | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/RootViewModel.js b/src/RootViewModel.js index 7c2d35e..be416df 100644 --- a/src/RootViewModel.js +++ b/src/RootViewModel.js @@ -36,19 +36,12 @@ export class RootViewModel extends ViewModel { } _updateChildVMs(oldLink) { - if (this.link) { - this.createLinkViewModel = null; - if (!oldLink || !oldLink.equals(this.link)) { - this.openLinkViewModel = new OpenLinkViewModel(this.childOptions({ - link: this.link, - clients: createClients(), - })); - } - } else { - this.openLinkViewModel = null; - this.createLinkViewModel = new CreateLinkViewModel(this.childOptions()); + if (!oldLink || !oldLink.equals(this.link)) { + this.openLinkViewModel = new OpenLinkViewModel(this.childOptions({ + link: this.link, + clients: createClients(), + })); } - this.emitChange(); } _hideLinks() { @@ -58,24 +51,30 @@ export class RootViewModel extends ViewModel { } updateHash(hash) { + // All view models except openLink are re-created anyway. + // Might as well clear them to avoid having to + // manually reset (n-1)/n view models in every case. + const oldLink = this.link; + this.link = null; this.showDisclaimer = false; + this.loadServerPolicyViewModel = null; + this.createLinkViewModel = null; if (hash.startsWith("#/policy/")) { const server = hash.substr(9); - this._hideLinks(); + this.openLinkViewModel = null; this.loadServerPolicyViewModel = new LoadServerPolicyViewModel(this.childOptions({server})); this.loadServerPolicyViewModel.load(); - this.emitChange(); } else if (hash.startsWith("#/disclaimer/")) { - this._hideLinks(); - this.loadServerPolicyViewModel = null; + this.openLinkViewModel = null; this.showDisclaimer = true; - this.emitChange(); - } else { - const oldLink = this.link; - this.loadServerPolicyViewModel = null; + } else if (hash === "" || hash === "#" || hash === "#/") { + this.openLinkViewModel = null; + this.createLinkViewModel = new CreateLinkViewModel(this.childOptions()); + } else if (this.link = Link.parse(hash)) { this.link = Link.parse(hash); this._updateChildVMs(oldLink); } + this.emitChange(); } clearPreferences() { From 628e99ba2d0c36deaee0d28837fcf2cc0947d1a3 Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Mon, 30 Aug 2021 11:08:58 -0700 Subject: [PATCH 2/8] Cleanup view switching code a bit --- src/RootViewModel.js | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/RootViewModel.js b/src/RootViewModel.js index be416df..2cc1070 100644 --- a/src/RootViewModel.js +++ b/src/RootViewModel.js @@ -35,10 +35,13 @@ export class RootViewModel extends ViewModel { }); } - _updateChildVMs(oldLink) { - if (!oldLink || !oldLink.equals(this.link)) { + _updateChildVMs(newLink, oldLink) { + this.link = newLink; + if (!newLink) { + this.openLinkViewModel = null; + } else if (!oldLink || !oldLink.equals(newLink)) { this.openLinkViewModel = new OpenLinkViewModel(this.childOptions({ - link: this.link, + link: newLink, clients: createClients(), })); } @@ -51,28 +54,27 @@ export class RootViewModel extends ViewModel { } updateHash(hash) { - // All view models except openLink are re-created anyway. - // Might as well clear them to avoid having to - // manually reset (n-1)/n view models in every case. + // All view models except openLink are re-created anyway. Might as well + // clear them to avoid having to manually reset (n-1)/n view models in every case. + // That just doesn't scale well when we add new views. const oldLink = this.link; - this.link = null; this.showDisclaimer = false; this.loadServerPolicyViewModel = null; this.createLinkViewModel = null; + let newLink; if (hash.startsWith("#/policy/")) { const server = hash.substr(9); - this.openLinkViewModel = null; + this._updateChildVMs(null, oldLink); this.loadServerPolicyViewModel = new LoadServerPolicyViewModel(this.childOptions({server})); this.loadServerPolicyViewModel.load(); } else if (hash.startsWith("#/disclaimer/")) { - this.openLinkViewModel = null; + this._updateChildVMs(null, oldLink); this.showDisclaimer = true; } else if (hash === "" || hash === "#" || hash === "#/") { - this.openLinkViewModel = null; + this._updateChildVMs(null, oldLink); this.createLinkViewModel = new CreateLinkViewModel(this.childOptions()); - } else if (this.link = Link.parse(hash)) { - this.link = Link.parse(hash); - this._updateChildVMs(oldLink); + } else if (newLink = Link.parse(hash)) { + this._updateChildVMs(newLink, oldLink); } this.emitChange(); } From e4aeadecd777a854b1bc3e3eec4a67f089f1e282 Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Mon, 30 Aug 2021 12:05:42 -0700 Subject: [PATCH 3/8] Add a 'invalid URL' view --- src/InvalidUrlView.js | 29 +++++++++++++++++++++++++++++ src/RootView.js | 2 ++ src/RootViewModel.js | 5 +++++ 3 files changed, 36 insertions(+) create mode 100644 src/InvalidUrlView.js diff --git a/src/InvalidUrlView.js b/src/InvalidUrlView.js new file mode 100644 index 0000000..e6a58e7 --- /dev/null +++ b/src/InvalidUrlView.js @@ -0,0 +1,29 @@ +/* +Copyright 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 {TemplateView} from "./utils/TemplateView.js"; + +export class InvalidUrlView extends TemplateView { + render(t) { + return t.div({ className: "DisclaimerView card" }, [ + t.h1("Invalid URL"), + t.p([ + 'The link you have entered is not valid. If you like, you can ', + t.a({ href: "#/" }, 'return to the home page.') + ]), + ]); + } +} diff --git a/src/RootView.js b/src/RootView.js index 5669f36..218ad57 100644 --- a/src/RootView.js +++ b/src/RootView.js @@ -19,10 +19,12 @@ import {OpenLinkView} from "./open/OpenLinkView.js"; import {CreateLinkView} from "./create/CreateLinkView.js"; import {LoadServerPolicyView} from "./policy/LoadServerPolicyView.js"; import {DisclaimerView} from "./disclaimer/DisclaimerView.js"; +import {InvalidUrlView} from "./InvalidUrlView.js"; export class RootView extends TemplateView { render(t, vm) { return t.div({className: "RootView"}, [ + t.mapView(vm => vm.invalidUrl, invalid => invalid ? new InvalidUrlView() : null), t.mapView(vm => vm.showDisclaimer, disclaimer => disclaimer ? new DisclaimerView() : null), t.mapView(vm => vm.openLinkViewModel, vm => vm ? new OpenLinkView(vm) : null), t.mapView(vm => vm.createLinkViewModel, vm => vm ? new CreateLinkView(vm) : null), diff --git a/src/RootViewModel.js b/src/RootViewModel.js index 2cc1070..b9cde3f 100644 --- a/src/RootViewModel.js +++ b/src/RootViewModel.js @@ -30,6 +30,7 @@ export class RootViewModel extends ViewModel { this.createLinkViewModel = null; this.loadServerPolicyViewModel = null; this.showDisclaimer = false; + this.invalidUrl = false; this.preferences.on("canClear", () => { this.emitChange(); }); @@ -58,6 +59,7 @@ export class RootViewModel extends ViewModel { // clear them to avoid having to manually reset (n-1)/n view models in every case. // That just doesn't scale well when we add new views. const oldLink = this.link; + this.invalidUrl = false; this.showDisclaimer = false; this.loadServerPolicyViewModel = null; this.createLinkViewModel = null; @@ -75,6 +77,9 @@ export class RootViewModel extends ViewModel { this.createLinkViewModel = new CreateLinkViewModel(this.childOptions()); } else if (newLink = Link.parse(hash)) { this._updateChildVMs(newLink, oldLink); + } else { + this._updateChildVMs(null, oldLink); + this.invalidUrl = true; } this.emitChange(); } From 0844279c58b8e828dc39e1776ce7af5348fc49ae Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Mon, 30 Aug 2021 12:11:21 -0700 Subject: [PATCH 4/8] Error on missing initial hash --- src/Link.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Link.js b/src/Link.js index 079d7be..53459ae 100644 --- a/src/Link.js +++ b/src/Link.js @@ -105,9 +105,10 @@ export class Link { webInstances = getWebInstanceMap(queryParams); } - if (linkStr.startsWith("#/")) { - linkStr = linkStr.substr(2); + if (!linkStr.startsWith("#/")) { + return null; } + linkStr = linkStr.substr(2); const [identifier, eventId] = linkStr.split("/"); From b57fbd8b6d213ece500847ed54d95ba43c5f90e1 Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Mon, 30 Aug 2021 14:14:31 -0700 Subject: [PATCH 5/8] Add suggestions for invalid URLs --- src/InvalidUrlView.js | 22 +++++++++++++++++++++- src/InvalidUrlViewModel.js | 25 +++++++++++++++++++++++++ src/Link.js | 18 ++++++++++++++++++ src/RootView.js | 2 +- src/RootViewModel.js | 9 ++++++--- 5 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 src/InvalidUrlViewModel.js diff --git a/src/InvalidUrlView.js b/src/InvalidUrlView.js index e6a58e7..a13bcd2 100644 --- a/src/InvalidUrlView.js +++ b/src/InvalidUrlView.js @@ -15,15 +15,35 @@ limitations under the License. */ import {TemplateView} from "./utils/TemplateView.js"; +import {LinkKind} from "./Link.js"; export class InvalidUrlView extends TemplateView { - render(t) { + render(t, vm) { return t.div({ className: "DisclaimerView card" }, [ t.h1("Invalid URL"), t.p([ 'The link you have entered is not valid. If you like, you can ', t.a({ href: "#/" }, 'return to the home page.') ]), + vm.validFixes.length ? this._renderValidFixes(t, vm.validFixes) : [], + ]); + } + + _describeLinkKind(kind) { + switch (kind) { + case LinkKind.Room: return "The room "; + case LinkKind.User: return "The user "; + case LinkKind.Group: return "The group "; + case LinkKind.Event: return "An event in room "; + } + } + + _renderValidFixes(t, validFixes) { + return t.p([ + 'Did you mean any of the following?', + t.ul(validFixes.map(fix => + t.li([this._describeLinkKind(fix.link.kind), t.a({ href: fix.url }, fix.link.identifier)]) + )) ]); } } diff --git a/src/InvalidUrlViewModel.js b/src/InvalidUrlViewModel.js new file mode 100644 index 0000000..dc0bf56 --- /dev/null +++ b/src/InvalidUrlViewModel.js @@ -0,0 +1,25 @@ +/* +Copyright 2020 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 {ViewModel} from "./utils/ViewModel.js"; +import {tryFixUrl} from "./Link.js"; + +export class InvalidUrlViewModel extends ViewModel { + constructor(options) { + super(options); + this.validFixes = tryFixUrl(options.fragment); + } +} diff --git a/src/Link.js b/src/Link.js index 53459ae..76bfa8a 100644 --- a/src/Link.js +++ b/src/Link.js @@ -71,6 +71,24 @@ export const LinkKind = createEnum( "Event" ) +export function tryFixUrl(fragment) { + const attempts = []; + if (fragment.startsWith('#') && !fragment.startsWith('#/')) { + if (!fragment.match(/^#[@$!]/)) { + attempts.push('#/' + fragment); // #room => #/#room + } + attempts.push('#/' + fragment.substring(1)); // #@room => #/@room + } + const validAttempts = []; + for (const attempt of attempts) { + const link = Link.parse(attempt); + if (link) { + validAttempts.push({ url: attempt, link }); + } + } + return validAttempts; +} + export class Link { static validateIdentifier(identifier) { return !!( diff --git a/src/RootView.js b/src/RootView.js index 218ad57..3dc2d23 100644 --- a/src/RootView.js +++ b/src/RootView.js @@ -24,7 +24,7 @@ import {InvalidUrlView} from "./InvalidUrlView.js"; export class RootView extends TemplateView { render(t, vm) { return t.div({className: "RootView"}, [ - t.mapView(vm => vm.invalidUrl, invalid => invalid ? new InvalidUrlView() : null), + t.mapView(vm => vm.invalidUrlViewModel, invalidVM => invalidVM ? new InvalidUrlView(invalidVM) : null), t.mapView(vm => vm.showDisclaimer, disclaimer => disclaimer ? new DisclaimerView() : null), t.mapView(vm => vm.openLinkViewModel, vm => vm ? new OpenLinkView(vm) : null), t.mapView(vm => vm.createLinkViewModel, vm => vm ? new CreateLinkView(vm) : null), diff --git a/src/RootViewModel.js b/src/RootViewModel.js index b9cde3f..39f6733 100644 --- a/src/RootViewModel.js +++ b/src/RootViewModel.js @@ -20,6 +20,7 @@ import {OpenLinkViewModel} from "./open/OpenLinkViewModel.js"; import {createClients} from "./open/clients/index.js"; import {CreateLinkViewModel} from "./create/CreateLinkViewModel.js"; import {LoadServerPolicyViewModel} from "./policy/LoadServerPolicyViewModel.js"; +import {InvalidUrlViewModel} from "./InvalidUrlViewModel.js"; import {Platform} from "./Platform.js"; export class RootViewModel extends ViewModel { @@ -29,8 +30,8 @@ export class RootViewModel extends ViewModel { this.openLinkViewModel = null; this.createLinkViewModel = null; this.loadServerPolicyViewModel = null; + this.invalidUrlViewModel = null; this.showDisclaimer = false; - this.invalidUrl = false; this.preferences.on("canClear", () => { this.emitChange(); }); @@ -59,7 +60,7 @@ export class RootViewModel extends ViewModel { // clear them to avoid having to manually reset (n-1)/n view models in every case. // That just doesn't scale well when we add new views. const oldLink = this.link; - this.invalidUrl = false; + this.invalidUrlViewModel = null; this.showDisclaimer = false; this.loadServerPolicyViewModel = null; this.createLinkViewModel = null; @@ -79,7 +80,9 @@ export class RootViewModel extends ViewModel { this._updateChildVMs(newLink, oldLink); } else { this._updateChildVMs(null, oldLink); - this.invalidUrl = true; + this.invalidUrlViewModel = new InvalidUrlViewModel(this.childOptions({ + fragment: hash + })); } this.emitChange(); } From e2fb5de5951ab8fdece50f41de2fc87959869845 Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Mon, 30 Aug 2021 14:39:56 -0700 Subject: [PATCH 6/8] Adjust fix suggestions --- src/Link.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Link.js b/src/Link.js index 76bfa8a..9621e02 100644 --- a/src/Link.js +++ b/src/Link.js @@ -73,14 +73,13 @@ export const LinkKind = createEnum( export function tryFixUrl(fragment) { const attempts = []; - if (fragment.startsWith('#') && !fragment.startsWith('#/')) { - if (!fragment.match(/^#[@$!]/)) { - attempts.push('#/' + fragment); // #room => #/#room - } - attempts.push('#/' + fragment.substring(1)); // #@room => #/@room - } + const afterHash = fragment.substring(fragment.startsWith("#/") ? 2 : 1); + attempts.push('#/@' + afterHash); // #room => #/@room + attempts.push('#/#' + afterHash); // #@room => #/@room + attempts.push('#/!' + afterHash); // #@room => #/@room + const validAttempts = []; - for (const attempt of attempts) { + for (const attempt of [...new Set(attempts)]) { const link = Link.parse(attempt); if (link) { validAttempts.push({ url: attempt, link }); From 5f5359ae652dd5afd949ce723e751c10fa1a7195 Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Mon, 30 Aug 2021 14:48:22 -0700 Subject: [PATCH 7/8] Remove outdated comments --- src/Link.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Link.js b/src/Link.js index 9621e02..cb7653f 100644 --- a/src/Link.js +++ b/src/Link.js @@ -74,9 +74,9 @@ export const LinkKind = createEnum( export function tryFixUrl(fragment) { const attempts = []; const afterHash = fragment.substring(fragment.startsWith("#/") ? 2 : 1); - attempts.push('#/@' + afterHash); // #room => #/@room - attempts.push('#/#' + afterHash); // #@room => #/@room - attempts.push('#/!' + afterHash); // #@room => #/@room + attempts.push('#/@' + afterHash); + attempts.push('#/#' + afterHash); + attempts.push('#/!' + afterHash); const validAttempts = []; for (const attempt of [...new Set(attempts)]) { From 380282955704166e605059ddcf368e8a2202f431 Mon Sep 17 00:00:00 2001 From: Danila Fedorin Date: Wed, 1 Sep 2021 10:11:19 -0700 Subject: [PATCH 8/8] Differentiate between rooms and room aliases --- src/InvalidUrlView.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/InvalidUrlView.js b/src/InvalidUrlView.js index a13bcd2..ecc66da 100644 --- a/src/InvalidUrlView.js +++ b/src/InvalidUrlView.js @@ -15,7 +15,7 @@ limitations under the License. */ import {TemplateView} from "./utils/TemplateView.js"; -import {LinkKind} from "./Link.js"; +import {LinkKind, IdentifierKind} from "./Link.js"; export class InvalidUrlView extends TemplateView { render(t, vm) { @@ -29,12 +29,16 @@ export class InvalidUrlView extends TemplateView { ]); } - _describeLinkKind(kind) { - switch (kind) { - case LinkKind.Room: return "The room "; + _describeRoom(identifierKind) { + return identifierKind === IdentifierKind.RoomAlias ? "room alias" : "room"; + } + + _describeLinkKind(linkKind, identifierKind) { + switch (linkKind) { + case LinkKind.Room: return `The ${this._describeRoom(identifierKind)} `; case LinkKind.User: return "The user "; case LinkKind.Group: return "The group "; - case LinkKind.Event: return "An event in room "; + case LinkKind.Event: return `An event in ${this._describeRoom(identifierKind)} `; } } @@ -42,7 +46,10 @@ export class InvalidUrlView extends TemplateView { return t.p([ 'Did you mean any of the following?', t.ul(validFixes.map(fix => - t.li([this._describeLinkKind(fix.link.kind), t.a({ href: fix.url }, fix.link.identifier)]) + t.li([ + this._describeLinkKind(fix.link.kind, fix.link.identifierKind), + t.a({ href: fix.url }, fix.link.identifier) + ]) )) ]); }