Merge pull request #354 from vector-im/bwindels/fix-encrypted-tiles

Fix crash when rendering non-decrypted message tiles

src/domain/session/room/timeline/MessageBody.js

@@ -0,0 +1,97 @@
+import { linkify } from "./linkify/linkify.js";
+
+export function parsePlainBody(body) {
+    const parts = [];
+    const lines = body.split("\n");
+
+    // create callback outside of loop
+    const linkifyCallback = (text, isLink) => {
+        if (isLink) {
+            parts.push(new LinkPart(text, text));
+        } else {
+            parts.push(new TextPart(text));
+        }
+    };
+
+    for (let i = 0; i < lines.length; i += 1) {
+        const line = lines[i];
+        if (line.length) {
+            linkify(lines[i], linkifyCallback);
+        }
+        const isLastLine = i >= (lines.length - 1);
+        if (!isLastLine) {
+            parts.push(new NewLinePart());
+        }
+    }
+
+    return new MessageBody(body, parts);
+}
+
+export function stringAsBody(body) {
+    return new MessageBody(body, [new TextPart(body)]);
+}
+
+class NewLinePart {
+    get type() { return "newline"; }
+}
+
+class LinkPart {
+    constructor(url, text) {
+        this.url = url;
+        this.text = text;
+    }
+
+    get type() { return "link"; }
+}
+
+class TextPart {
+    constructor(text) {
+        this.text = text;
+    }
+
+    get type() { return "text"; }
+}
+
+class MessageBody {
+    constructor(sourceString, parts) {
+        this.sourceString = sourceString;
+        this.parts = parts;
+    }
+}
+
+export function tests() {
+
+    function test(assert, input, output) {
+        assert.deepEqual(parsePlainBody(input), new MessageBody(input, output));
+    }
+
+    return {
+        // Tests for text
+        "Text only": assert => {
+            const input = "This is a sentence";
+            const output = [new TextPart(input)];
+            test(assert, input, output);
+        },
+
+        "Text with newline": assert => {
+            const input = "This is a sentence.\nThis is another sentence.";
+            const output = [
+                new TextPart("This is a sentence."),
+                new NewLinePart(),
+                new TextPart("This is another sentence.")
+            ];
+            test(assert, input, output);
+        },
+
+        "Text with newline & trailing newline": assert => {
+            const input = "This is a sentence.\nThis is another sentence.\n";
+            const output = [
+                new TextPart("This is a sentence."),
+                new NewLinePart(),
+                new TextPart("This is another sentence."),
+                new NewLinePart()
+            ];
+            test(assert, input, output);
+        }
+    };
+}

src/domain/session/room/timeline/MessageBodyBuilder.js

@@ -1,96 +0,0 @@
-import { linkify } from "./linkify/linkify.js";
-
-export class MessageBodyBuilder {
-
-    constructor(message = []) {
-        this._root = message;
-    }
-
-    fromText(text) {
-        const components = text.split("\n");
-        components.flatMap(e => ["\n", e]).slice(1).forEach(e => {
-            if (e === "\n") {
-                this.insertNewline();
-            }
-            else {
-                linkify(e, this.insert.bind(this));
-            }
-        });
-    }
-
-    insert(text, isLink) {
-        if (!text.length) {
-            return;
-        }
-        if (isLink) {
-            this.insertLink(text, text);
-        }
-        else {
-            this.insertText(text);
-        }
-    }
-
-    insertText(text) {
-        if (text.length) {
-            this._root.push({ type: "text", text: text });
-        }
-    }
-
-    insertLink(link, displayText) {
-        this._root.push({ type: "link", url: link, text: displayText });
-    }
-
-    insertNewline() {
-        this._root.push({ type: "newline" });
-    }
-
-    [Symbol.iterator]() {
-        return this._root.values();
-    }
-
-}
-
-export function tests() {
-
-    function linkify(text) {
-        const obj = new MessageBodyBuilder();
-        obj.fromText(text);
-        return obj;
-    }
-
-    function test(assert, input, output) {
-        output = new MessageBodyBuilder(output);
-        input = linkify(input);
-        assert.deepEqual(input, output);
-    }
-
-    return {
-        // Tests for text
-        "Text only": assert => {
-            const input = "This is a sentence";
-            const output = [{ type: "text", text: input }];
-            test(assert, input, output);
-        },
-
-        "Text with newline": assert => {
-            const input = "This is a sentence.\nThis is another sentence.";
-            const output = [
-                { type: "text", text: "This is a sentence." },
-                { type: "newline" },
-                { type: "text", text: "This is another sentence." }
-            ];
-            test(assert, input, output);
-        },
-
-        "Text with newline & trailing newline": assert => {
-            const input = "This is a sentence.\nThis is another sentence.\n";
-            const output = [
-                { type: "text", text: "This is a sentence." },
-                { type: "newline" },
-                { type: "text", text: "This is another sentence." },
-                { type: "newline" }
-            ];
-            test(assert, input, output);
-        }
-    };
-}

src/domain/session/room/timeline/tiles/BaseMediaTile.js

@@ -15,12 +15,12 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import {MessageTile} from "./MessageTile.js";
+import {BaseMessageTile} from "./BaseMessageTile.js";
 import {SendStatus} from "../../../../../matrix/room/sending/PendingEvent.js";
 const MAX_HEIGHT = 300;
 const MAX_WIDTH = 400;
 
-export class BaseMediaTile extends MessageTile {
+export class BaseMediaTile extends BaseMessageTile {
     constructor(options) {
         super(options);
         this._decryptedThumbnail = null;

src/domain/session/room/timeline/tiles/BaseMessageTile.js

@@ -17,7 +17,7 @@ limitations under the License.
 import {SimpleTile} from "./SimpleTile.js";
 import {getIdentifierColorNumber, avatarInitials, getAvatarHttpUrl} from "../../../../avatar.js";
 
-export class MessageTile extends SimpleTile {
+export class BaseMessageTile extends SimpleTile {
     constructor(options) {
         super(options);
         this._isOwn = this._entry.sender === options.ownUserId;
@@ -33,10 +33,6 @@ export class MessageTile extends SimpleTile {
         return this._room.mediaRepository;
     }
 
-    get shape() {
-        return "message";
-    }
-
     get displayName() {
         return this._entry.displayName || this.sender;
     }
@@ -89,7 +85,7 @@ export class MessageTile extends SimpleTile {
     updatePreviousSibling(prev) {
         super.updatePreviousSibling(prev);
         let isContinuation = false;
-        if (prev && prev instanceof MessageTile && prev.sender === this.sender) {
+        if (prev && prev instanceof BaseMessageTile && prev.sender === this.sender) {
             // timestamp is null for pending events
             const myTimestamp = this._entry.timestamp || this.clock.now();
             const otherTimestamp = prev._entry.timestamp || this.clock.now();

src/domain/session/room/timeline/tiles/BaseTextTile.js

@@ -0,0 +1,47 @@
+/*
+Copyright 2020 Bruno Windels <bruno@windels.cloud>
+
+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 {BaseMessageTile} from "./BaseMessageTile.js";
+import {stringAsBody} from "../MessageBody.js";
+
+export class BaseTextTile extends BaseMessageTile {
+    constructor(options) {
+        super(options);
+        this._messageBody = null;
+    }
+
+    get shape() {
+        return "message";
+    }
+
+    _parseBody(bodyString) {
+        return stringAsBody(bodyString);
+    }
+
+    get body() {
+        const body = this._getBodyAsString();
+        // body is a string, so we can check for difference by just
+        // doing an equality check
+        if (!this._messageBody || this._messageBody.sourceString !== body) {
+            // body with markup is an array of parts,
+            // so we should not recreate it for the same body string,
+            // or else the equality check in the binding will always fail.
+            // So cache it here.
+            this._messageBody = this._parseBody(body);
+        }
+        return this._messageBody;
+    }
+}

src/domain/session/room/timeline/tiles/EncryptedEventTile.js

@@ -14,10 +14,10 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import {MessageTile} from "./MessageTile.js";
+import {BaseTextTile} from "./BaseTextTile.js";
 import {UpdateAction} from "../UpdateAction.js";
 
-export class EncryptedEventTile extends MessageTile {
+export class EncryptedEventTile extends BaseTextTile {
     updateEntry(entry, params) {
         const parentResult = super.updateEntry(entry, params);
         // event got decrypted, recreate the tile and replace this one with it
@@ -33,7 +33,7 @@ export class EncryptedEventTile extends MessageTile {
         return "message-status"
     }
 
-    get text() {
+    _getBodyAsString() {
         const decryptionError = this._entry.decryptionError;
         const code = decryptionError?.code;
         if (code === "MEGOLM_NO_SESSION") {

src/domain/session/room/timeline/tiles/FileTile.js

@@ -15,11 +15,11 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import {MessageTile} from "./MessageTile.js";
+import {BaseMessageTile} from "./BaseMessageTile.js";
 import {formatSize} from "../../../../../utils/formatSize.js";
 import {SendStatus} from "../../../../../matrix/room/sending/PendingEvent.js";
 
-export class FileTile extends MessageTile {
+export class FileTile extends BaseMessageTile {
     constructor(options) {
         super(options);
         this._downloadError = null;

src/domain/session/room/timeline/tiles/LocationTile.js

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import {MessageTile} from "./MessageTile.js";
+import {BaseMessageTile} from "./BaseMessageTile.js";
 
 /*
 map urls:
@@ -23,7 +23,7 @@ android: https://developers.google.com/maps/documentation/urls/guide
 wp:      maps:49.275267 -122.988617
 https://www.habaneroconsulting.com/stories/insights/2011/opening-native-map-apps-from-the-mobile-browser
 */
-export class LocationTile extends MessageTile {
+export class LocationTile extends BaseMessageTile {
     get mapsLink() {
         const geoUri = this._getContent().geo_uri;
         const [lat, long] = geoUri.split(":")[1].split(",");

src/domain/session/room/timeline/tiles/MissingAttachmentTile.js

@@ -14,9 +14,9 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import {MessageTile} from "./MessageTile.js";
+import {BaseMessageTile} from "./BaseMessageTile.js";
 
-export class MissingAttachmentTile extends MessageTile {
+export class MissingAttachmentTile extends BaseMessageTile {
     get shape() {
         return "missing-attachment"
     }

src/domain/session/room/timeline/tiles/SimpleTile.js

@@ -31,7 +31,7 @@ export class SimpleTile extends ViewModel {
     }
 
     // don't show display name / avatar
-    // probably only for MessageTiles of some sort?
+    // probably only for BaseMessageTiles of some sort?
     get isContinuation() {
         return false;
     }

src/domain/session/room/timeline/tiles/TextTile.js

@@ -14,12 +14,11 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import {MessageTile} from "./MessageTile.js";
+import {BaseTextTile} from "./BaseTextTile.js";
-import { MessageBodyBuilder } from "../MessageBodyBuilder.js";
+import {parsePlainBody} from "../MessageBody.js";
 
-export class TextTile extends MessageTile {
+export class TextTile extends BaseTextTile {
-
+    _getBodyAsString() {
-    get _contentBody() {
         const content = this._getContent();
         let body = content?.body || "";
         if (content.msgtype === "m.emote") {
@@ -28,14 +27,7 @@ export class TextTile extends MessageTile {
         return body;
     }
 
-    get body() {
+    _parseBody(bodyString) {
-        const body = this._contentBody;
+        return parsePlainBody(bodyString);
-        if (body === this._body) {
-            return this._message;
-        }
-        const message = new MessageBodyBuilder();
-        message.fromText(body);
-        [this._body, this._message] = [body, message];
-        return message;
     }
 }

src/platform/web/ui/session/room/timeline/TextMessageView.js

@@ -16,7 +16,7 @@ limitations under the License.
 
 import {TemplateView} from "../../../general/TemplateView.js";
 import {StaticView} from "../../../general/StaticView.js";
-import { tag, text } from "../../../general/html.js";
+import {tag, text} from "../../../general/html.js";
 import {renderMessage} from "./common.js";
 
 export class TextMessageView extends TemplateView {
@@ -29,19 +29,19 @@ export class TextMessageView extends TemplateView {
 }
 
 const formatFunction = {
-    text: (m) => text(m.text),
+    text: textPart => text(textPart.text),
-    link: (m) => tag.a({ href: m.url, target: "_blank", rel: "noopener" }, [text(m.text)]),
+    link: linkPart => tag.a({ href: linkPart.url, target: "_blank", rel: "noopener" }, [linkPart.text]),
     newline: () => tag.br()
 };
 
 class BodyView extends StaticView {
-    render(t, value) {
+    render(t, messageBody) {
-        const children = [];
+        const container = t.span();
-        for (const m of value) {
+        for (const part of messageBody.parts) {
-            const f = formatFunction[m.type];
+            const f = formatFunction[part.type];
-            const element = f(m);
+            const element = f(part);
-            children.push(element);
+            container.appendChild(element);
         }
-        return t.span(children);
+        return container;
     }
 }