OrchardCore icon indicating copy to clipboard operation
OrchardCore copied to clipboard

Markdown rich editor corrupts Arabic text in OC 1.4

Open dodyg opened this issue 1 year ago • 27 comments

Describe the bug

The Arabic text in this rich editor is corrupted

mde-markdown

This is the text on the same exact field without the rich editor

mde-markdown-1

dodyg avatar Aug 10 '22 14:08 dodyg

Have you tried directly with the same text and the markdown editor on their website demo?

Skrypt avatar Aug 10 '22 14:08 Skrypt

Good idea let me try it

dodyg avatar Aug 10 '22 15:08 dodyg

This is the corrupted data in EasyMDE

corrupt

This is the actual rendering

https://try.orchardcore.net/wmuhsxod/blog/post-1

The website is https://try.orchardcore.net/wmuhsxod Username: admin Password: ?B002t-L

If anyone wants to replicate

dodyg avatar Aug 10 '22 15:08 dodyg

برنامج مؤسسة ساويرس للحصول على الدبلوم الفني الفندقي من المدرسة الألمانية الفندقية بالجونة

There is no problem with the EasyMDE itself (https://easy-markdown-editor.tk/)

easy-mark

But at OC14, the text becomes corrupted easy-mark-2

dodyg avatar Aug 10 '22 15:08 dodyg

The editor in OC 1.4 is the same version as https://easy-markdown-editor.tk/ easy-mark-3

dodyg avatar Aug 10 '22 15:08 dodyg

I see the text is reversed in the editor. That would be a styling issue.

Skrypt avatar Aug 10 '22 15:08 Skrypt

I just found this repository which probably indicates that EasyMDE has issues with RTL. https://github.com/imAbdelhadi/easymde-rtl

Skrypt avatar Aug 10 '22 15:08 Skrypt

@dodyg I think we could bundle the RTL version in OC and switch to it based on CultureInfo RTL. Feel free to open a pull request.

Skrypt avatar Aug 10 '22 15:08 Skrypt

Seems I come late to fix the Arabic issue ;) from the first time I saw the text, the letters in the words have been revered, so we could fix this as @Skrypt suggested before

@dodyg is it working before OC 1.4 or the issue from the beginning because no one tried Arabic text ;)

hishamco avatar Aug 10 '22 17:08 hishamco

@hishamco I can't remember but the site was already English/Arabic since OC 1.0 and this was the first complain regarding reverse text.

dodyg avatar Aug 11 '22 07:08 dodyg

OC-3

@hishamco OK this is another site of mine running on OC 1.3. It works fine here.

dodyg avatar Aug 11 '22 07:08 dodyg

Maybe a BS 5 upgrade regression. If this can be fixed by changing only styles then it will be better. I didn't find where it is reversing the text.

Skrypt avatar Aug 11 '22 07:08 Skrypt

I might look into this

hishamco avatar Aug 11 '22 20:08 hishamco

The culprit is in TheAdmin.css

style-1 style-2

dodyg avatar Aug 13 '22 11:08 dodyg

I found the problem.

ltr here problem-1

Changing it to rtl fix it. problem-2

dodyg avatar Aug 13 '22 11:08 dodyg

@hishamco

This is most likely the problematic css in TheAdmin.css file. There is a comment about rtl:ignore. I am not sure what the reason is. Maybe people more familiar with the theme can review this.

html[data-theme=default] pre,
html[data-theme=default] code,
html[data-theme=default] kbd,
html[data-theme=default] samp {
  font-family: var(--bs-font-monospace);
  font-size: 1em;
  direction: ltr /* rtl:ignore */;
  unicode-bidi: bidi-override;
}

https://raw.githubusercontent.com/OrchardCMS/OrchardCore/main/src/OrchardCore.Themes/TheAdmin/wwwroot/css/TheAdmin.css

dodyg avatar Aug 13 '22 11:08 dodyg

I think from the style author, unless @Skrypt did it for a reason

hishamco avatar Aug 13 '22 11:08 hishamco

bidi

The direction is really not necessary if the unicode-bidi: bidi-override; is removed.

  direction: ltr /* rtl:ignore */;
  unicode-bidi: bidi-override;

dodyg avatar Aug 13 '22 15:08 dodyg

This thing is like hydra. Once the unicode-bidi removed, the codemirror.min.css needs to be dealt with.

code-mirror

dodyg avatar Aug 13 '22 16:08 dodyg

One possible solution is changing

html[data-theme=default] pre,
html[data-theme=default] code,
html[data-theme=default] kbd,
html[data-theme=default] samp {
  font-family: var(--bs-font-monospace);
  font-size: 1em;
  direction: ltr /* rtl:ignore */;
  unicode-bidi: bidi-override;
}

to


html[data-theme=default] pre,
html[data-theme=default] code,
html[data-theme=default] kbd,
html[data-theme=default] samp {
  font-family: var(--bs-font-monospace);
  font-size: 1em;
}

html[dir=ltr][data-theme=default] pre,
html[dir=ltr][data-theme=default] code,
html[dir=ltr][data-theme=default] kbd,
html[dir=ltr][data-theme=default] samp {
    direction: ltr !important;
}

html[dir=rtl][data-theme=default] pre,
html[dir=rtl][data-theme=default] code,
html[dir=rtl][data-theme=default] kbd,
html[dir=rtl][data-theme=default] samp {
    direction: rtl !important;
}

dodyg avatar Aug 13 '22 16:08 dodyg

Above solution works on Microsoft Edge

edge

Firefox firefox

but not Google Chrome

chrome

dodyg avatar Aug 15 '22 12:08 dodyg

Have you tried removing only the /*rtl:ignore*/ comment? Also, make sure you test with different RTL cultures afterward because it seems like it works for other people.

Skrypt avatar Aug 16 '22 11:08 Skrypt

Hebrew is also reversed

hebrew-1 hebrew-2

Site: https://try.orchardcore.net/2z5kxzxk username: admin password: _Y73koqP

dodyg avatar Aug 16 '22 12:08 dodyg

This is how the Hebrew posting supposed to be. I had to modify the RTL directly.

supposed-to-be

I am not sure what's the purpose of removing /*rtl:ignore*/ because it doesn't really show up at the inspector.

dodyg avatar Aug 16 '22 12:08 dodyg

I don't understand why the unicode-bidi: bidi-override; is required. Because if you remove this, the direction can be removed.


html[data-theme=default] pre,
html[data-theme=default] code,
html[data-theme=default] kbd,
html[data-theme=default] samp {
  font-family: var(--bs-font-monospace);
  font-size: 1em;
  direction: ltr /* rtl:ignore */;
  unicode-bidi: bidi-override;
}

dodyg avatar Aug 16 '22 12:08 dodyg

Good question. The issue is that it needs to be tested with more than just the Markdown editor. These classes are applied to base HTML tags. That means that at some point we decided to override the direction of these texts and also remove the unicode-bidirectional ability and force it to be LTR.

So, the proper fix would be to use a CSS class that wraps around the Markdown editor only and that will override these styles that affect it. And also, keep these base HTML styles because they seems to be appropriate. You generally don't want to reverse a <code></code> example in a page.

Skrypt avatar Aug 16 '22 12:08 Skrypt

Yeah that makes sense. Let me tinker with this later so it only targets EasyMDE.

dodyg avatar Aug 16 '22 12:08 dodyg