element-hq / element-web

A glossy Matrix collaboration client for the web.

https://element.io

GNU Affero General Public License v3.0

11.14k stars 1.98k forks source link

Emoji rendering destroys paragraphs #6523

Closed spantaleev closed 6 years ago

spantaleev commented 6 years ago

This seems to be a regression since 0.14.0.

The paragraphs in the following message get destroyed while rendering.

Original (as it appears in the message input box):

test 🙂

test

Resulting event:

  "content": {
    "body": "test 🙂\n\ntest",
    "msgtype": "m.text"
  },

Incorrectly renders as:

riot-web-emoji-plain

Workaround 1

Forcing Markdown-processing to trigger works around this problem.

The following message:

**test** 🙂

test

produces the following event, which renders correctly (paragraphs are preserved):

  "content": {
    "body": "**test** 🙂\n\ntest",
    "msgtype": "m.text",
    "formatted_body": "<p><strong>test</strong> 🙂</p>\n<p>test</p>\n",
    "format": "org.matrix.custom.html"
  },

Workaround 2

If there's no emoji in the text, everything works well.

The following message:

test

test

produces the following event, which renders correctly (paragraphs are preserved):

  "content": {
    "body": "test\n\ntest",
    "msgtype": "m.text"
  },

lukebarnard1 commented 6 years ago

We apply the CSS class markdown-body in messages with emoji (because we insert emoji with HTML). This class includes the rule white-space: normal;.

With white-space: normal, "Sequences of whitespace are collapsed..." including newline characters.

Using white-space: pre-wrap (preserves \n as <br>) might be a solution (which is what .mx_MTextBody defines) , but it would be worth:

testing this in a whole world of different cases
finding out why we're using white-space: normal (perhaps something to do with wrapping? or overflowing?)
writing a bunch of unit tests for EventTile :hearts:

zeratax commented 6 years ago

Same seems to happen for Braille Patterns (U+2800-U+28FF) possibly more.

spantaleev commented 6 years ago

Will this see some fix/workaround in 0.14.x?

Seems like a pretty annoying regression to drag along.

MazeChaZer commented 6 years ago

I found the problem that caused this bug, will submit a pull request this evening.

MazeChaZer commented 6 years ago

See matrix-org/matrix-react-sdk#1910

MazeChaZer commented 6 years ago

@ZerataX The pull request also fixes the problem for braille characters

MazeChaZer commented 6 years ago

@lukebarnard1 I removed the class markdown-body from emoji messages because it looked like it was accidentally added in matrix-org/matrix-react-sdk@4f4441fb07dcdfb831e4b54b9f8d7e611c172f29, so it shouldn't be a problem. Does that look good to you?

lukebarnard1 commented 6 years ago

It does indeed, merged :smiley: