Fix alignment of RTL messages (#12837)

* Fix alignment of RTL messages

Inspired by #5453 but
hopefully with the edited marker in the right place.

This is a PoC: types aren't correct and the style needs pulling
out to a class. Plus it would probably need more visual tests added.
If this looks acceptable, I can make these changes.

* Fix spacing between text and edited annotation

* Update snapshot

* Update more snapshots

* More snapshots

* More more snapshots

* Split out style

* Fix emotes

This will cause them always be right-justified if the display name
is rtl.

* Add playwright test for ltr/rtl message rendering

* Better snapshots

* Await on message sending

* Better waiting, hopefully

* Old snapshot files

* Really hopefully fixed screenshots this time

* Don't include the message action bar in the screenshots

playwright/e2e/messages/messages.spec.ts

@@ -0,0 +1,113 @@
+/*
+Copyright 2024 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.
+*/
+
+/* See readme.md for tips on writing these tests. */
+
+import { Locator, Page } from "playwright-core";
+
+import { test, expect } from "../../element-web-test";
+
+async function sendMessage(page: Page, message: string): Promise<Locator> {
+    await page.getByRole("textbox", { name: "Send a message…" }).fill(message);
+    await page.getByRole("button", { name: "Send message" }).click();
+
+    const msgTile = await page.locator(".mx_EventTile_last");
+    await msgTile.locator(".mx_EventTile_receiptSent").waitFor();
+    return msgTile;
+}
+
+async function editMessage(page: Page, message: Locator, newMsg: string): Promise<void> {
+    const line = message.locator(".mx_EventTile_line");
+    await line.hover();
+    await line.getByRole("button", { name: "Edit" }).click();
+    const editComposer = page.getByRole("textbox", { name: "Edit message" });
+    await page.getByLabel("User menu").hover(); // Just to un-hover the message line
+    await editComposer.fill(newMsg);
+    await editComposer.press("Enter");
+}
+
+test.describe("Message rendering", () => {
+    [
+        { direction: "ltr", displayName: "Quentin" },
+        { direction: "rtl", displayName: "كوينتين" },
+    ].forEach(({ direction, displayName }) => {
+        test.describe(`with ${direction} display name`, () => {
+            test.use({
+                displayName,
+                room: async ({ user, app }, use) => {
+                    const roomId = await app.client.createRoom({ name: "Test room" });
+                    await use({ roomId });
+                },
+            });
+
+            test("should render a basic LTR text message", async ({ page, user, app, room }) => {
+                await page.goto(`#/room/${room.roomId}`);
+
+                const msgTile = await sendMessage(page, "Hello, world!");
+                await expect(msgTile).toMatchScreenshot(`basic-message-ltr-${direction}displayname.png`, {
+                    mask: [page.locator(".mx_MessageTimestamp")],
+                });
+            });
+
+            test("should render an LTR emote", async ({ page, user, app, room }) => {
+                await page.goto(`#/room/${room.roomId}`);
+
+                const msgTile = await sendMessage(page, "/me lays an egg");
+                await expect(msgTile).toMatchScreenshot(`emote-ltr-${direction}displayname.png`);
+            });
+
+            test("should render an edited LTR message", async ({ page, user, app, room }) => {
+                await page.goto(`#/room/${room.roomId}`);
+
+                const msgTile = await sendMessage(page, "Hello, world!");
+
+                await editMessage(page, msgTile, "Hello, universe!");
+
+                await expect(msgTile).toMatchScreenshot(`edited-message-ltr-${direction}displayname.png`, {
+                    mask: [page.locator(".mx_MessageTimestamp")],
+                });
+            });
+
+            test("should render a basic RTL text message", async ({ page, user, app, room }) => {
+                await page.goto(`#/room/${room.roomId}`);
+
+                const msgTile = await sendMessage(page, "مرحبا بالعالم!");
+                await expect(msgTile).toMatchScreenshot(`basic-message-rtl-${direction}displayname.png`, {
+                    mask: [page.locator(".mx_MessageTimestamp")],
+                });
+            });
+
+            test("should render an RTL emote", async ({ page, user, app, room }) => {
+                await page.goto(`#/room/${room.roomId}`);
+
+                const msgTile = await sendMessage(page, "/me يضع بيضة");
+                await expect(msgTile).toMatchScreenshot(`emote-rtl-${direction}displayname.png`);
+            });
+
+            test("should render an edited RTL message", async ({ page, user, app, room }) => {
+                await page.goto(`#/room/${room.roomId}`);
+
+                const msgTile = await sendMessage(page, "مرحبا بالعالم!");
+
+                await editMessage(page, msgTile, "مرحبا بالكون!");
+
+                await expect(msgTile).toMatchScreenshot(`edited-message-rtl-${direction}displayname.png`, {
+                    mask: [page.locator(".mx_MessageTimestamp")],
+                });
+            });
+        });
+    });
+});

res/css/views/messages/_MEmoteBody.pcss

@@ -16,6 +16,7 @@ limitations under the License.
 
 .mx_MEmoteBody {
     white-space: pre-wrap;
+    text-align: start;
 }
 
 .mx_MEmoteBody_sender {

res/css/views/rooms/_EventTile.pcss

@@ -43,6 +43,7 @@ $left-gutter: 64px;
 
     .mx_EventTile_body {
         overflow-y: hidden;
+        text-align: start;
     }
 
     .mx_EventTile_receiptSent,
@@ -676,7 +677,7 @@ $left-gutter: 64px;
         font-size: $font-12px;
         color: $secondary-content;
         display: inline-block;
-        margin-left: 9px;
+        margin-inline-start: 9px;
     }
 
     .mx_EventTile_edited {
@@ -1443,6 +1444,10 @@ $left-gutter: 64px;
     }
 }
 
+.mx_EventTile_annotated {
+    display: flex;
+}
+
 /* Media query for mobile UI */
 @media only screen and (max-width: 480px) {
     .mx_EventTile_content {

src/HtmlUtils.tsx

@@ -303,7 +303,6 @@ export interface EventRenderOpts {
     disableBigEmoji?: boolean;
     stripReplyFallback?: boolean;
     forComposerQuote?: boolean;
-    ref?: React.Ref<HTMLSpanElement>;
 }
 
 function analyseEvent(content: IContent, highlights: Optional<string[]>, opts: EventRenderOpts = {}): EventAnalysis {
@@ -375,7 +374,61 @@ function analyseEvent(content: IContent, highlights: Optional<string[]>, opts: E
     }
 }
 
-export function bodyToNode(content: IContent, highlights: Optional<string[]>, opts: EventRenderOpts = {}): ReactNode {
+export function bodyToDiv(
+    content: IContent,
+    highlights: Optional<string[]>,
+    opts: EventRenderOpts = {},
+    ref?: React.Ref<HTMLDivElement>,
+): ReactNode {
+    const { strippedBody, formattedBody, emojiBodyElements, className } = bodyToNode(content, highlights, opts);
+
+    return formattedBody ? (
+        <div
+            key="body"
+            ref={ref}
+            className={className}
+            dangerouslySetInnerHTML={{ __html: formattedBody }}
+            dir="auto"
+        />
+    ) : (
+        <div key="body" ref={ref} className={className} dir="auto">
+            {emojiBodyElements || strippedBody}
+        </div>
+    );
+}
+
+export function bodyToSpan(
+    content: IContent,
+    highlights: Optional<string[]>,
+    opts: EventRenderOpts = {},
+    ref?: React.Ref<HTMLSpanElement>,
+    includeDir = true,
+): ReactNode {
+    const { strippedBody, formattedBody, emojiBodyElements, className } = bodyToNode(content, highlights, opts);
+
+    return formattedBody ? (
+        <span
+            key="body"
+            ref={ref}
+            className={className}
+            dangerouslySetInnerHTML={{ __html: formattedBody }}
+            dir={includeDir ? "auto" : undefined}
+        />
+    ) : (
+        <span key="body" ref={ref} className={className} dir={includeDir ? "auto" : undefined}>
+            {emojiBodyElements || strippedBody}
+        </span>
+    );
+}
+
+interface BodyToNodeReturn {
+    strippedBody: string;
+    formattedBody?: string;
+    emojiBodyElements: JSX.Element[] | undefined;
+    className: string;
+}
+
+function bodyToNode(content: IContent, highlights: Optional<string[]>, opts: EventRenderOpts = {}): BodyToNodeReturn {
     const eventInfo = analyseEvent(content, highlights, opts);
 
     let emojiBody = false;
@@ -419,19 +472,7 @@ export function bodyToNode(content: IContent, highlights: Optional<string[]>, op
         emojiBodyElements = formatEmojis(eventInfo.strippedBody, false) as JSX.Element[];
     }
 
-    return formattedBody ? (
-        <span
-            key="body"
-            ref={opts.ref}
-            className={className}
-            dangerouslySetInnerHTML={{ __html: formattedBody }}
-            dir="auto"
-        />
-    ) : (
-        <span key="body" ref={opts.ref} className={className} dir="auto">
-            {emojiBodyElements || eventInfo.strippedBody}
-        </span>
-    );
+    return { strippedBody: eventInfo.strippedBody, formattedBody, emojiBodyElements, className };
 }
 
 /**

src/components/views/messages/EditHistoryMessage.tsx

@@ -172,7 +172,7 @@ export default class EditHistoryMessage extends React.PureComponent<IProps, ISta
             if (this.props.previousEdit) {
                 contentElements = editBodyDiffToHtml(getReplacedContent(this.props.previousEdit), content);
             } else {
-                contentElements = HtmlUtils.bodyToNode(content, null, {
+                contentElements = HtmlUtils.bodyToSpan(content, null, {
                     stripReplyFallback: true,
                 });
             }

src/components/views/messages/TextualBody.tsx

@@ -59,7 +59,7 @@ interface IState {
 }
 
 export default class TextualBody extends React.Component<IBodyProps, IState> {
-    private readonly contentRef = createRef<HTMLSpanElement>();
+    private readonly contentRef = createRef<HTMLDivElement>();
 
     private unmounted = false;
     private pills: Element[] = [];
@@ -566,34 +566,38 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {
         }
         const mxEvent = this.props.mxEvent;
         const content = mxEvent.getContent();
-        let isNotice = false;
-        let isEmote = false;
+        const isNotice = content.msgtype === MsgType.Notice;
+        const isEmote = content.msgtype === MsgType.Emote;
+
+        const willHaveWrapper =
+            this.props.replacingEventId || this.props.isSeeingThroughMessageHiddenForModeration || isEmote;
 
         // only strip reply if this is the original replying event, edits thereafter do not have the fallback
         const stripReply = !mxEvent.replacingEvent() && !!getParentEventId(mxEvent);
-        isEmote = content.msgtype === MsgType.Emote;
-        isNotice = content.msgtype === MsgType.Notice;
-        let body = HtmlUtils.bodyToNode(content, this.props.highlights, {
+
+        const htmlOpts = {
             disableBigEmoji: isEmote || !SettingsStore.getValue<boolean>("TextualBody.enableBigEmoji"),
             // Part of Replies fallback support
             stripReplyFallback: stripReply,
-            ref: this.contentRef,
-        });
+        };
+        let body = willHaveWrapper
+            ? HtmlUtils.bodyToSpan(content, this.props.highlights, htmlOpts, this.contentRef, false)
+            : HtmlUtils.bodyToDiv(content, this.props.highlights, htmlOpts, this.contentRef);
 
         if (this.props.replacingEventId) {
             body = (
-                <>
+                <div dir="auto" className="mx_EventTile_annotated">
                     {body}
                     {this.renderEditedMarker()}
-                </>
+                </div>
             );
         }
         if (this.props.isSeeingThroughMessageHiddenForModeration) {
             body = (
-                <>
+                <div dir="auto" className="mx_EventTile_annotated">
                     {body}
                     {this.renderPendingModerationMarker()}
-                </>
+                </div>
             );
         }
 
@@ -624,7 +628,7 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {
 
         if (isEmote) {
             return (
-                <div className="mx_MEmoteBody mx_EventTile_content" onClick={this.onBodyLinkClick}>
+                <div className="mx_MEmoteBody mx_EventTile_content" onClick={this.onBodyLinkClick} dir="auto">
                     *&nbsp;
                     <span className="mx_MEmoteBody_sender" onClick={this.onEmoteSenderClick}>
                         {mxEvent.sender ? mxEvent.sender.name : mxEvent.getSender()}

test/HtmlUtils-test.tsx

@@ -19,7 +19,7 @@ import { mocked } from "jest-mock";
 import { render, screen } from "@testing-library/react";
 import { IContent } from "matrix-js-sdk/src/matrix";
 
-import { bodyToNode, formatEmojis, topicToHtml } from "../src/HtmlUtils";
+import { bodyToSpan, formatEmojis, topicToHtml } from "../src/HtmlUtils";
 import SettingsStore from "../src/settings/SettingsStore";
 
 jest.mock("../src/settings/SettingsStore");
@@ -66,7 +66,7 @@ describe("topicToHtml", () => {
 
 describe("bodyToHtml", () => {
     function getHtml(content: IContent, highlights?: string[]): string {
-        return (bodyToNode(content, highlights, {}) as ReactElement).props.dangerouslySetInnerHTML.__html;
+        return (bodyToSpan(content, highlights, {}) as ReactElement).props.dangerouslySetInnerHTML.__html;
     }
 
     it("should apply highlights to HTML messages", () => {
@@ -108,14 +108,14 @@ describe("bodyToHtml", () => {
     });
 
     it("generates big emoji for emoji made of multiple characters", () => {
-        const { asFragment } = render(bodyToNode({ body: "👨‍👩‍👧‍👦 ↔️ 🇮🇸", msgtype: "m.text" }, [], {}) as ReactElement);
+        const { asFragment } = render(bodyToSpan({ body: "👨‍👩‍👧‍👦 ↔️ 🇮🇸", msgtype: "m.text" }, [], {}) as ReactElement);
 
         expect(asFragment()).toMatchSnapshot();
     });
 
     it("should generate big emoji for an emoji-only reply to a message", () => {
         const { asFragment } = render(
-            bodyToNode(
+            bodyToSpan(
                 {
                     "body": "> <@sender1:server> Test\n\n🥰",
                     "format": "org.matrix.custom.html",
@@ -139,7 +139,7 @@ describe("bodyToHtml", () => {
     });
 
     it("does not mistake characters in text presentation mode for emoji", () => {
-        const { asFragment } = render(bodyToNode({ body: "↔ ❗︎", msgtype: "m.text" }, [], {}) as ReactElement);
+        const { asFragment } = render(bodyToSpan({ body: "↔ ❗︎", msgtype: "m.text" }, [], {}) as ReactElement);
 
         expect(asFragment()).toMatchSnapshot();
     });