element-hq / element-android

A Matrix collaboration client for Android.
https://element.io/
GNU Affero General Public License v3.0
3.38k stars 728 forks source link

Composer: text looses formatting after editing a markdown message or editing a reply message #8602

Open Luka5W opened 1 year ago

Luka5W commented 1 year ago

Steps to reproduce

wrong formatting when editing a message:

  1. enable markdown messages
  2. send any message, e.g.
    # test
    > foo
  3. the message is sent correctly
  4. edit the message - the message is rendered in the composer like in the chat (with big header and a cite) but the formatting marks are missing. in my case i just added a /n/n´bar´ (i was not able to break/ exit the comment either)
  5. submit message
  6. the new edited message is (without the cite):

    test <- not a header

    foo <- not a cite

    bar <- that works because i added the formatting marks

    if you add the formatting marks manually before submitting the edit, the message is sent correctly.

1

duplicate in reply to …:

when replying to any message and editing the reply the same bug occures:

  1. markdown can be enabled but it is not required
  2. send any message
  3. reply to that message with any message
  4. edit the repy - the message is rendered with the

    in reply to @… *initial message*

    *reply message* which is applied to the message after submitting the edit draft.

  5. the edited message is:

    in reply to @… *initial message*

    in reply to @… *initial message*

    *reply message*

2

there might be much more bugs when editing messages i haven't found.

Outcome

What did you expect?

the message is rendered correctly

What happened instead?

i believe the problem is that the rendered message text is put into the composer instead of the raw message text so all formatting of the message gets lost when editing

Your phone model

OnePlus 5T

Operating system version

Android 10

Application version and app store

Element version v1.6.5 from Google Play

Homeserver

Synapse v1.89.0

Will you send logs?

No

Are you willing to provide a PR?

No

Luka5W commented 1 year ago

@julioromano why did you add the tags a-rich-text-editor and o-uncommon? I haven't used the rich text editor but the "old"/ current one. And I believe editing messages isn't an unexpected workflow - or did I get the tag descriptions wrong?

Anyway, I somehow figured out where the problem might be (Haven't worked in big projects yet and I'm not good in Kotlin). I'm sure, Yoan has thought anything when implementing those lines: im.vector.app.features.home.room.detail.composer.PlainTextComposerLayout.kt#L212-L216 in #8377.

When I commented out the lines the app behaved as I expected but I haven't tested it very well so other functionality may be broken.

So what should I do? Should I revert the lines and get the 'ugly' but efficient (and especially correct) markdown formatting back or leave it as it is and fix the problem elsewhere? (idk, maybe the class which receives the text after editing)

Luka5W commented 1 year ago

why did you add the tags a-rich-text-editor and o-uncommon? I haven't used the rich text editor but the "old"/ current one. And I believe editing messages isn't an unexpected workflow - or did I get the tag descriptions wrong?

My fault. I know the difference between the composer and rich text editor now.

julioromano commented 1 year ago

why did you add the tags a-rich-text-editor and o-uncommon? I haven't used the rich text editor but the "old"/ current one. And I believe editing messages isn't an unexpected workflow - or did I get the tag descriptions wrong?

My fault. I know the difference between the composer and rich text editor now.

Apologies. I set the correct tag now. Occurrence is uncommon because most users don't write markdown so won't come across this behavior.

Luka5W commented 1 year ago

why did you add the tags a-rich-text-editor and o-uncommon? I haven't used the rich text editor but the "old"/ current one. And I believe editing messages isn't an unexpected workflow - or did I get the tag descriptions wrong?

My fault. I know the difference between the composer and rich text editor now.

Apologies. I set the correct tag now. Occurrence is uncommon because most users don't write markdown so won't come across this behavior.

what? and you are telling me most users dont reply to messages and editing the replies afterwards?! (yes, theyre affected too. see the 2nd screenshot) the whole composer is fucked up because of #8377 FOR MONTHS which was implemented without any QA or else! (and because of other minor markdown bugs like escaping, lists, etc. but i dont care about them since theyre not too annoying)

tags are still wrong btw. please remove the a-markdown tag and add the a-composer

julioromano commented 1 year ago

why did you add the tags a-rich-text-editor and o-uncommon? I haven't used the rich text editor but the "old"/ current one. And I believe editing messages isn't an unexpected workflow - or did I get the tag descriptions wrong?

My fault. I know the difference between the composer and rich text editor now.

Apologies. I set the correct tag now. Occurrence is uncommon because most users don't write markdown so won't come across this behavior.

what? and you are telling me most users dont reply to messages and editing the replies afterwards?! (yes, theyre affected too. see the 2nd screenshot) the whole composer is fucked up because of #8377 FOR MONTHS which was implemented without any QA or else! (and because of other minor markdown bugs like escaping, lists, etc. but i dont care about them since theyre not too annoying)

tags are still wrong btw. please remove the a-markdown tag and add the a-composer

Thanks for the additional explanations, tags should be ok now. Btw if you strongly think #8377 is the culprit you might want to submit a bugfix PR.

Luka5W commented 1 year ago

uhm... serious question... are you trolling me?

@Luka5W Luka5W linked a pull request Sep 2, 2023 that will close this issue Bugfix: Composer: text looses formatting after editing a markdown message or editing a reply message #8638

julioromano commented 1 year ago

uhm... serious question... are you trolling me?

@Luka5W Luka5W linked a pull request Sep 2, 2023 that will close this issue Bugfix: Composer: text looses formatting after editing a markdown message or editing a reply message #8638

No trolls here. Thanks, I'll bring that PR up to the team's attention.

Luka5W commented 1 year ago

well, it just looked like… anyway, thanks a lot.

Luka5W commented 1 year ago

Thanks, I'll bring that PR up to the team's attention.

@julioromano

What did the team say?

I've thought someone was assigned to the PR, but he wasn't.

The longer we're waiting the more difficult it may become to merge...

julioromano commented 1 year ago

Thanks, I'll bring that PR up to the team's attention.

@julioromano

What did the team say?

I've thought someone was assigned to the PR, but he wasn't.

The longer we're waiting the more difficult it may become to merge...

Hello, Whilst this issue has gotten the team's attention, we don't have enough bandwidth to tackle it right now. I've bumped the severity to better reflect the issue.

Luka5W commented 1 year ago

That's great since it isn't a big change and shouldn't take too much time to verify - I believe

It's rather meant as a hot fix. No offense, but the whole composer/ message rendering would need a redesign because there are some bugs (e.g. the Markdown rendering is different compared to the element-desktop/ -web clients). But since you are mainly working on Element-X a rewrite wouldn't make any sense of course. So this PR is good enough to last until Element X is finished I guess...

Anyway, thank you for pushing the PR.