From 01b8b397b63116923406ebcaf3358269720a1374 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 May 2021 11:26:30 +0200 Subject: [PATCH 1/6] expose sourceString on result of parsing message body and also do some cleanup --- .../session/room/timeline/MessageBody.js | 105 ++++++++++++++++++ .../room/timeline/MessageBodyBuilder.js | 96 ---------------- 2 files changed, 105 insertions(+), 96 deletions(-) create mode 100644 src/domain/session/room/timeline/MessageBody.js delete mode 100644 src/domain/session/room/timeline/MessageBodyBuilder.js diff --git a/src/domain/session/room/timeline/MessageBody.js b/src/domain/session/room/timeline/MessageBody.js new file mode 100644 index 00000000..a88d39c8 --- /dev/null +++ b/src/domain/session/room/timeline/MessageBody.js @@ -0,0 +1,105 @@ +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; + } + + get sourceString() { + return this._sourceString; + } + + get parts() { + return this._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); + } + }; +} diff --git a/src/domain/session/room/timeline/MessageBodyBuilder.js b/src/domain/session/room/timeline/MessageBodyBuilder.js deleted file mode 100644 index f1b34462..00000000 --- a/src/domain/session/room/timeline/MessageBodyBuilder.js +++ /dev/null @@ -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); - } - }; -} From 054c51b82f5663b5cc7c10d08e6ef0a21a3eeb9c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 May 2021 11:27:30 +0200 Subject: [PATCH 2/6] add caching MessageBody in BaseTextMessageTile,use in EncryptedEventTile missing body in EncryptedEventTile was what caused the bug --- .../timeline/tiles/BaseTextMessageTile.js | 47 +++++++++++++++++++ .../room/timeline/tiles/EncryptedEventTile.js | 6 +-- .../room/timeline/tiles/MessageTile.js | 4 -- .../session/room/timeline/tiles/TextTile.js | 20 +++----- 4 files changed, 56 insertions(+), 21 deletions(-) create mode 100644 src/domain/session/room/timeline/tiles/BaseTextMessageTile.js diff --git a/src/domain/session/room/timeline/tiles/BaseTextMessageTile.js b/src/domain/session/room/timeline/tiles/BaseTextMessageTile.js new file mode 100644 index 00000000..e92e326a --- /dev/null +++ b/src/domain/session/room/timeline/tiles/BaseTextMessageTile.js @@ -0,0 +1,47 @@ +/* +Copyright 2020 Bruno Windels + +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 {MessageTile} from "./MessageTile.js"; +import {stringAsBody} from "../MessageBody.js"; + +export class BaseTextMessageTile extends MessageTile { + 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; + } +} diff --git a/src/domain/session/room/timeline/tiles/EncryptedEventTile.js b/src/domain/session/room/timeline/tiles/EncryptedEventTile.js index 23476ebb..ceb771c2 100644 --- a/src/domain/session/room/timeline/tiles/EncryptedEventTile.js +++ b/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 {BaseTextMessageTile} from "./BaseTextMessageTile.js"; import {UpdateAction} from "../UpdateAction.js"; -export class EncryptedEventTile extends MessageTile { +export class EncryptedEventTile extends BaseTextMessageTile { 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") { diff --git a/src/domain/session/room/timeline/tiles/MessageTile.js b/src/domain/session/room/timeline/tiles/MessageTile.js index f85f0fca..1769748d 100644 --- a/src/domain/session/room/timeline/tiles/MessageTile.js +++ b/src/domain/session/room/timeline/tiles/MessageTile.js @@ -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; } diff --git a/src/domain/session/room/timeline/tiles/TextTile.js b/src/domain/session/room/timeline/tiles/TextTile.js index 88e281d2..fdfdd32f 100644 --- a/src/domain/session/room/timeline/tiles/TextTile.js +++ b/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 { MessageBodyBuilder } from "../MessageBodyBuilder.js"; +import {BaseTextMessageTile} from "./BaseTextMessageTile.js"; +import {parsePlainBody} from "../MessageBody.js"; -export class TextTile extends MessageTile { - - get _contentBody() { +export class TextTile extends BaseTextMessageTile { + _getBodyAsString() { 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() { - const body = this._contentBody; - if (body === this._body) { - return this._message; - } - const message = new MessageBodyBuilder(); - message.fromText(body); - [this._body, this._message] = [body, message]; - return message; + _parseBody(bodyString) { + return parsePlainBody(bodyString); } } From fa64fcce2df2d24005caa91a1f663a7c6a6056e2 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 May 2021 11:29:00 +0200 Subject: [PATCH 3/6] expect MessageBody here with parts property and do some cleanup --- .../session/room/timeline/TextMessageView.js | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/platform/web/ui/session/room/timeline/TextMessageView.js b/src/platform/web/ui/session/room/timeline/TextMessageView.js index b9eda412..4af5e5a2 100644 --- a/src/platform/web/ui/session/room/timeline/TextMessageView.js +++ b/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), - link: (m) => tag.a({ href: m.url, target: "_blank", rel: "noopener" }, [text(m.text)]), + text: textPart => text(textPart.text), + link: linkPart => tag.a({ href: linkPart.url, target: "_blank", rel: "noopener" }, [linkPart.text]), newline: () => tag.br() }; class BodyView extends StaticView { - render(t, value) { - const children = []; - for (const m of value) { - const f = formatFunction[m.type]; - const element = f(m); - children.push(element); + render(t, messageBody) { + const container = t.span(); + for (const part of messageBody.parts) { + const f = formatFunction[part.type]; + const element = f(part); + container.appendChild(element); } - return t.span(children); + return container; } } From 67714040e735565c161aabe2467bf38dafb23e21 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 May 2021 12:14:15 +0200 Subject: [PATCH 4/6] rename BaseTextMessageTile to BaseTextTile --- .../tiles/{BaseTextMessageTile.js => BaseTextTile.js} | 4 ++-- src/domain/session/room/timeline/tiles/EncryptedEventTile.js | 4 ++-- src/domain/session/room/timeline/tiles/TextTile.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) rename src/domain/session/room/timeline/tiles/{BaseTextMessageTile.js => BaseTextTile.js} (93%) diff --git a/src/domain/session/room/timeline/tiles/BaseTextMessageTile.js b/src/domain/session/room/timeline/tiles/BaseTextTile.js similarity index 93% rename from src/domain/session/room/timeline/tiles/BaseTextMessageTile.js rename to src/domain/session/room/timeline/tiles/BaseTextTile.js index e92e326a..401fd719 100644 --- a/src/domain/session/room/timeline/tiles/BaseTextMessageTile.js +++ b/src/domain/session/room/timeline/tiles/BaseTextTile.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 {BaseMessageTile} from "./BaseMessageTile.js"; import {stringAsBody} from "../MessageBody.js"; -export class BaseTextMessageTile extends MessageTile { +export class BaseTextTile extends BaseMessageTile { constructor(options) { super(options); this._messageBody = null; diff --git a/src/domain/session/room/timeline/tiles/EncryptedEventTile.js b/src/domain/session/room/timeline/tiles/EncryptedEventTile.js index ceb771c2..155ce4a9 100644 --- a/src/domain/session/room/timeline/tiles/EncryptedEventTile.js +++ b/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 {BaseTextMessageTile} from "./BaseTextMessageTile.js"; +import {BaseTextTile} from "./BaseTextTile.js"; import {UpdateAction} from "../UpdateAction.js"; -export class EncryptedEventTile extends BaseTextMessageTile { +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 diff --git a/src/domain/session/room/timeline/tiles/TextTile.js b/src/domain/session/room/timeline/tiles/TextTile.js index fdfdd32f..07e7ce8f 100644 --- a/src/domain/session/room/timeline/tiles/TextTile.js +++ b/src/domain/session/room/timeline/tiles/TextTile.js @@ -14,10 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ -import {BaseTextMessageTile} from "./BaseTextMessageTile.js"; +import {BaseTextTile} from "./BaseTextTile.js"; import {parsePlainBody} from "../MessageBody.js"; -export class TextTile extends BaseTextMessageTile { +export class TextTile extends BaseTextTile { _getBodyAsString() { const content = this._getContent(); let body = content?.body || ""; From ce976226f9f0bd72b520e3159338be056a80d18b Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 May 2021 12:15:13 +0200 Subject: [PATCH 5/6] rename MessageTile to BaseMessageTile since MessageTile doesn't have a shape property anymore --- src/domain/session/room/timeline/tiles/BaseMediaTile.js | 4 ++-- .../timeline/tiles/{MessageTile.js => BaseMessageTile.js} | 4 ++-- src/domain/session/room/timeline/tiles/FileTile.js | 4 ++-- src/domain/session/room/timeline/tiles/LocationTile.js | 4 ++-- .../session/room/timeline/tiles/MissingAttachmentTile.js | 4 ++-- src/domain/session/room/timeline/tiles/SimpleTile.js | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) rename src/domain/session/room/timeline/tiles/{MessageTile.js => BaseMessageTile.js} (95%) diff --git a/src/domain/session/room/timeline/tiles/BaseMediaTile.js b/src/domain/session/room/timeline/tiles/BaseMediaTile.js index 26862902..e5e62107 100644 --- a/src/domain/session/room/timeline/tiles/BaseMediaTile.js +++ b/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; diff --git a/src/domain/session/room/timeline/tiles/MessageTile.js b/src/domain/session/room/timeline/tiles/BaseMessageTile.js similarity index 95% rename from src/domain/session/room/timeline/tiles/MessageTile.js rename to src/domain/session/room/timeline/tiles/BaseMessageTile.js index 1769748d..a348a249 100644 --- a/src/domain/session/room/timeline/tiles/MessageTile.js +++ b/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; @@ -85,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(); diff --git a/src/domain/session/room/timeline/tiles/FileTile.js b/src/domain/session/room/timeline/tiles/FileTile.js index 5770f7e0..c761e68e 100644 --- a/src/domain/session/room/timeline/tiles/FileTile.js +++ b/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; diff --git a/src/domain/session/room/timeline/tiles/LocationTile.js b/src/domain/session/room/timeline/tiles/LocationTile.js index 58870361..fddc4501 100644 --- a/src/domain/session/room/timeline/tiles/LocationTile.js +++ b/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(","); diff --git a/src/domain/session/room/timeline/tiles/MissingAttachmentTile.js b/src/domain/session/room/timeline/tiles/MissingAttachmentTile.js index 0a9b5976..847901ac 100644 --- a/src/domain/session/room/timeline/tiles/MissingAttachmentTile.js +++ b/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" } diff --git a/src/domain/session/room/timeline/tiles/SimpleTile.js b/src/domain/session/room/timeline/tiles/SimpleTile.js index f63fde74..ea5640bd 100644 --- a/src/domain/session/room/timeline/tiles/SimpleTile.js +++ b/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; } From 645470cd038dae63be461183558fe21d25ca3198 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 17 May 2021 12:45:55 +0200 Subject: [PATCH 6/6] no need for private prop here --- src/domain/session/room/timeline/MessageBody.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/domain/session/room/timeline/MessageBody.js b/src/domain/session/room/timeline/MessageBody.js index a88d39c8..a2250dfb 100644 --- a/src/domain/session/room/timeline/MessageBody.js +++ b/src/domain/session/room/timeline/MessageBody.js @@ -54,16 +54,8 @@ class TextPart { class MessageBody { constructor(sourceString, parts) { - this._sourceString = sourceString; - this._parts = parts; - } - - get sourceString() { - return this._sourceString; - } - - get parts() { - return this._parts; + this.sourceString = sourceString; + this.parts = parts; } }