Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Luka5W opened this issue Aug 6, 2023 · 11 comments
Labels
A-Composer O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Something isn't working: bugs, crashes, hangs and other reported problems

Comments

@Luka5W
Copy link

Luka5W commented Aug 6, 2023

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 Luka5W added the T-Defect Something isn't working: bugs, crashes, hangs and other reported problems label Aug 6, 2023
@Luka5W Luka5W changed the title RTE: text looses formatting after editing a markdown message RTE: text looses formatting after editing a markdown message or editing a reply message Aug 6, 2023
@julioromano julioromano added the A-Rich-Text-Editor Issues with the new rich text editor, also known as the WYSIWYG editor label Aug 7, 2023
@github-actions github-actions bot added the Z-Labs label Aug 7, 2023
@julioromano julioromano added O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist labels Aug 7, 2023
Luka5W pushed a commit to Luka5W/element-android that referenced this issue Sep 1, 2023
Luka5W added a commit to Luka5W/element-android that referenced this issue Sep 1, 2023
@Luka5W
Copy link
Author

Luka5W commented Sep 1, 2023

@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 Luka5W changed the title RTE: text looses formatting after editing a markdown message or editing a reply message Composer: text looses formatting after editing a markdown message or editing a reply message Sep 2, 2023
@Luka5W
Copy link
Author

Luka5W commented Sep 2, 2023

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 julioromano added A-Markdown and removed A-Rich-Text-Editor Issues with the new rich text editor, also known as the WYSIWYG editor labels Sep 12, 2023
@github-actions github-actions bot removed the Z-Labs label Sep 12, 2023
@julioromano
Copy link

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
Copy link
Author

Luka5W commented Sep 13, 2023

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 julioromano added A-Composer O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience and removed A-Markdown O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Sep 13, 2023
@julioromano
Copy link

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
Copy link
Author

Luka5W commented Sep 13, 2023

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
Copy link

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
Copy link
Author

Luka5W commented Sep 14, 2023

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

@Luka5W
Copy link
Author

Luka5W commented Oct 20, 2023

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 julioromano added S-Major Severely degrades major functionality or product features, with no satisfactory workaround and removed S-Minor Impairs non-critical functionality or suitable workarounds exist labels Oct 20, 2023
@julioromano
Copy link

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
Copy link
Author

Luka5W commented Oct 20, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Composer O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect Something isn't working: bugs, crashes, hangs and other reported problems
Projects
Status: Important Issues & Topics (P1)
2 participants