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

RTL marks in displaynames affect following text #1712

Closed
matrixbot opened this issue Jul 1, 2016 · 9 comments · Fixed by matrix-org/matrix-js-sdk#1992
Closed

RTL marks in displaynames affect following text #1712

matrixbot opened this issue Jul 1, 2016 · 9 comments · Fixed by matrix-org/matrix-js-sdk#1992
Assignees
Labels
A-Timeline I18n O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect

Comments

@matrixbot
Copy link

Created by @ richvdh:sw1v.org.

Do we need to wrap them with <span dir=auto> or something?

@richvdh
Copy link
Member

richvdh commented Jul 1, 2016

<div class="mx_TextualEvent">@magnap:matrix.magnap.dk changed their display name from &#8238; to Magnap</div>

->

@magnap:matrix.magnap.dk changed their display name from ‮ to Magnap

@richvdh richvdh changed the title RTL marks in displaynames are allowed to affect following text RTL marks in displaynames affect following text Jul 1, 2016
@ara4n ara4n added T-Defect P2 S-Minor Impairs non-critical functionality or suitable workarounds exist labels Jul 31, 2016
@uhoreg
Copy link
Member

uhoreg commented Dec 21, 2016

FWIW, <span style="display:inline-block"> seems to also work in blocking the RTL mark from affecting subsequent text. The benefit of this over <span dir=auto> is that you can do something like <span class="user_alias"> and put the "display: inline-block" in the stylesheet, and also do additional styling if you wanted to. But it's more verbose (and unclear what it's for).

@richvdh
Copy link
Member

richvdh commented Jan 11, 2017

We need a solution not involving markup for this, because the messages also end up in pop-up notifications, which are plain text. As per my question at https://stackoverflow.com/questions/41487035/handling-right-to-left-left-to-right-override-characters-in-user-input, the right thing to do is probably to strip out RLO and LRO characters.

@richvdh
Copy link
Member

richvdh commented Aug 9, 2017

This is particularly bad where it affects collapsed joins/parts (MELS).

@afranke
Copy link
Contributor

afranke commented Dec 8, 2019

Also affects email notifications.

@t3chguy t3chguy mentioned this issue Jul 16, 2020
@jryans jryans removed the A-I18n label Mar 5, 2021
@SafaAlfulaij
Copy link
Contributor

Please look at my comment here:
element-hq/riot-android#270 (comment)
This is probably the best way to achieve this without stripping out characters.
Of course, this would go in matrix-react-sdk, not here.

The only downside is minimum browsers support (they will show as squares if the browser doesn't support those characters).

@ShadowJonathan
Copy link
Contributor

@SafaAlfulaij please also note that riot-android has been deprecated and replaced by https://github.com/vector-im/element-android

@SafaAlfulaij
Copy link
Contributor

@ShadowJonathan Fully aware, and using Element for Android
I just had my report on Riot :)
Thanks though!

@germain-gg germain-gg added O-Uncommon Most users are unlikely to come across this or unexpected workflow P2 A-Timeline and removed P2 labels Sep 22, 2021
@germain-gg germain-gg added O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience and removed O-Uncommon Most users are unlikely to come across this or unexpected workflow P2 labels Sep 30, 2021
@dbkr dbkr self-assigned this Oct 19, 2021
@dbkr dbkr added this to the App Team 1.9.4 milestone Oct 19, 2021
@dbkr
Copy link
Member

dbkr commented Oct 20, 2021

I've had a look at this and I think I agree with rich & his stackoverflow that stripping out RLO and LRO is the correct solution. This should still allow for either ltr/rtl display names and even displaynames with one embedded in the other. The only alternative I can see would be to add PDF characters to the end, but that also means counting how many you'd need.

dbkr added a commit to matrix-org/matrix-js-sdk that referenced this issue Oct 20, 2021
Strip RLO and LRO characters from name and rawDisplayName so they
can safely be embedded into other text without messing up the text
ordering.

Fixes element-hq/element-web#1712
dbkr added a commit to matrix-org/matrix-js-sdk that referenced this issue Oct 20, 2021
Strip RLO and LRO characters from name and rawDisplayName so they
can safely be embedded into other text without messing up the text
ordering.

Fixes element-hq/element-web#1712
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this issue Nov 8, 2021
* Mark old verification methods as deprecated ([\matrix-org#1994](matrix-org#1994)).
* Try to set a sender on search result events if possible ([\matrix-org#2004](matrix-org#2004)).
* Port some changes from group calls branch to develop ([\matrix-org#2001](matrix-org#2001)). Contributed by @SimonBrandner.
* Fetch room membership from server rather than relying on stored data ([\matrix-org#1998](matrix-org#1998)).
* Add method to fetch the MSC3266 Room Summary of a Room ([\matrix-org#1988](matrix-org#1988)).
* Don't show `Unable to access microphone` when cancelling screensharing dialog ([\matrix-org#2005](matrix-org#2005)). Fixes element-hq/element-web#19533 and element-hq/element-web#19533. Contributed by @SimonBrandner.
* Strip direction override characters from display names ([\matrix-org#1992](matrix-org#1992)). Fixes element-hq/element-web#1712 and element-hq/element-web#1712.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Timeline I18n O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect
Projects
None yet
Development

Successfully merging a pull request may close this issue.