S->M: newlines are not translated in HTML formatted_body

[auscompgeek]

Describe the bug If a Slack user sends a multiline formatted message, the newline is kept literally in the Matrix HTML formatted_body, rather than being translated to <br>.

To Reproduce

1. Send a multiline Slack message with italics, such as:

   I don't _actually_ want to trigger this bug,
   but I need some steps to reproduce.
   

2. Look at the Matrix message in a client that supports HTML, which will show simple whitespace rather than a newline.

Expected behavior The message should be on multiple lines.

[andrewshadura]

Confirmed: also works incorrectly in the other direction, newlines are simply chomped away.

[stephen304]

Would the solution be adding an extra substitution somewhere around https://github.com/matrix-org/matrix-appservice-slack/blob/develop/src/substitutions.ts#L111 ?

I'm not sure what exactly needs to be done - it looks to me like both incoming slack messages and native matrix messages have \n as newlines in the message source, only that the newlines from slack don't seem to work. Maybe the message types are not exactly the same causing the difference?

[andrewshadura]

I'd like to add that a double newline on the Matrix side translates into a single newline in Slack.

On Sun, 3 Oct 2021, at 18:25, Stephen Smith wrote:

Would the solution be somewhere around https://github.com/matrix-org/matrix-appservice-slack/blob/develop/src/substitutions.ts#L111 ?

I'm not sure what exactly needs to be done - it looks to me like both incoming slack messages and native matrix messages have \n as newlines in the message source, only that the newlines from slack don't seem to work. Maybe the message types are not exactly the same causing the difference?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/matrix-org/matrix-appservice-slack/issues/547#issuecomment-932982880, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLQBJ3MCCBUW3D27XBBOLUFB7WPANCNFSM4UNBJUBA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Cheers, Andrej

[auscompgeek]

@andrewshadura I think you're just describing #342, but one piece of information at a time.

[auscompgeek]

Would the solution be adding an extra substitution somewhere around https://github.com/matrix-org/matrix-appservice-slack/blob/develop/src/substitutions.ts#L111 ?

That's the matrixToSlack method. You'd want the other way around to fix this bug.

As the label suggests though, this could be fixed by using a proper parser.

[andrewshadura]

@andrewshadura I think you're just describing #342, but one piece of information at a time.

Indeed, sorry for the noise 🙂

[stephen304]

I realized that messages sent in matrix only use \n when the message is only m.text, but if it's m.text and format type org.matrix.custom.html, then it uses <br>. The bridged messages are also org.matrix.custom.html so I think this patch should work:

diff --git a/src/substitutions.ts b/src/substitutions.ts
index a59ac32..02eb2ae 100644
--- a/src/substitutions.ts
+++ b/src/substitutions.ts
@@ -64,6 +64,7 @@ class Substitutions {
         body = body.replace("<!channel>", "@room");
         body = body.replace("<!here>", "@room");
         body = body.replace("<!everyone>", "@room");
+        body = body.replace("\\n", "<br>");
 
         // if we have a file, attempt to get the direct link to the file
         if (file && file.permalink_public && file.url_private && file.permalink) {
diff --git a/tests/unit/substitutionsTest.ts b/tests/unit/substitutionsTest.ts
index d52688f..9ace22b 100644
--- a/tests/unit/substitutionsTest.ts
+++ b/tests/unit/substitutionsTest.ts
@@ -297,6 +297,17 @@ describe("Substitutions", () => {
         });
     });
 
+    describe("slackToMatrix", () => {
+        it("should replace <!channel> with @room", async () => {
+            const res = await substitutions.slackToMatrix("Hello <!channel>");
+            expect(res).to.equal("Hello @room");
+        });
+        it("should replace \\n with <br>", async () => {
+            const res = await substitutions.slackToMatrix("Hello\\nWorld");
+            expect(res).to.equal("Hello<br>World");
+        });
+    });
+
     // describe("slackTextToMatrixHTML", () => {
     //     it("should repeat a plain string", async () => {
     //         const res = await substitutions.slackTextToMatrixHTML("Hello World!");

I think that might work, assuming the \n coming from slack is a literal backslash and n character and isn't an actual newline being rendered as \n.

I know very little about running matrix stuff so I don't have a homeserver to test this (It seems like you have to have your own in order to run this bridge?)

Edit: I gave running a homeserver + bridge a shot - it seems dendrite is easier to get up and running but I haven't managed to get the bot to accept my invite to the control room and I get a M_USER_IN_USE error.