#421 Add experiment function RTL support by enyaxu · Pull Request #439 · marktext/marktext

enyaxu · 2018-07-25T07:06:35Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
New tests added?	not needed
Fixed tickets	#421
License	MIT

Description

Add experiment function for text direction RTL support

--

fxha

@enyaxu Is it normal that the scrollbar is on the left?

fxha · 2018-07-25T17:50:25Z

    :style="{ 'color': theme === 'dark' ? darkColor : lightColor, 'lineHeight': lineHeight, 'fontSize': fontSize,
-    'font-family': editorFontFamily ? `${editorFontFamily}, ${defaultFontFamily}` : `${defaultFontFamily}` }"
+    'font-family': editorFontFamily ? `${editorFontFamily}, ${defaultFontFamily}` : `${defaultFontFamily}`,
+    'direction': textDirection }"


I think we should use the HTML dir attribute instead of css style. Please see https://stackoverflow.com/a/5375907.

@fxha OK, I'll use dir attribute for this. Any suggestion about scrollbar position?

@mrg0lden @mohsenkhanpour What's the default scrollbar side? Is it on the left or right side?

@fxha @enyaxu The scrollbar is on the default right side on most (almost all) of rtl pages that I have seen. Similar to ltr.
Right is also better because it gives you a unified experience when working with texts of several languages.
You don't want to look again for your scrollbar when you switch between files written in different languages.

Jocs · 2018-07-26T02:03:48Z

Nice PR, great thanks for you , I'll review this evening.

enyaxu · 2018-07-26T07:49:58Z

Yes，when set up **dir** attitube to **rtl**, It’s deafult change sildebar position to left!

mohsenkhanpour · 2018-07-26T08:37:00Z

Does using the HTML dir attribute instead of css style also change the position of the slidebar and scrollbar?

Jocs · 2018-07-26T08:38:46Z

@mohsenkhanpour You can pull down this PR, and test locally.

fxha · 2018-07-26T08:45:04Z

Yes，when set up dir attitube to rtl, It’s deafult change sildebar position to left!

@enyaxu I think you can set the scrollbar direction via css to the right.

mohsenkhanpour · 2018-07-26T11:32:35Z

@Jocs @fxha @enyaxu
I downloaded the PR to test and here is what I found:
✅ 1. First of all the text itself is aligned to right as it should and adding English words doesn't break the style.

✅ 2. The scrollbar is to the left and the slidebar is to the left.

It doesn't look unusual at all. ✅

❎ 3. The line direction argument applies to the next opened file rather than the open document.

❎ 4. The source code viewer still in ltr, (this can be done and very useful if implemented)

❎ 5. No padding makes the first words in each line difficult to read:

Jocs · 2018-07-26T13:14:08Z

@enyaxu @mohsenkhanpour @fxha

The source code viewer still in ltr, (this can be done and very useful if implemented)

Maybe you need to look up codemirror's documents and to see weather it support rtl and ltr options.

enyaxu · 2018-07-26T14:06:55Z

@Jocs @mohsenkhanpour
I pull a request for source code viewer RTL support. please test, Thanks.

enyaxu · 2018-07-26T14:22:43Z

@mohsenkhanpour
Can you upload markdown file for me used for testing?

enyaxu · 2018-07-26T14:29:38Z

@Jocs @fxha

The line direction argument applies to the next opened file rather than the open document.

For now I'm reading the source code. I can't find a way to attach and save attribute with each single file. So I just used preference to save textDirection value. This cause all document default applying textDirection value when changed.

enyaxu · 2018-07-26T14:34:01Z

@Jocs @fxha @mohsenkhanpour

The scrollbar is to the left and the slidebar is to the left.

Should we need to change this to left or remain it?

Jocs · 2018-07-27T15:04:00Z

I pull a request for source code viewer RTL support. please test, Thanks.

ok

Should we need to change this to left or remain it?

I usually don't use right-to-left writing, so I'm not sure about the position of the scrollbar. I can only give some suggestions on the code. can @mohsenkhanpour give some advice?

Jocs · 2018-07-27T16:01:45Z

@enyaxu

For now I'm reading the source code. I can't find a way to attach and save attribute with each single file. So I just used preference to save textDirection value. This cause all document default applying textDirection value when changed.

As we discussed in the issue before, the textdirection in the edit menu only affects the current document, should not be written to the user preferences, and this does not need to be stored. I suggest you refer to the implementation of line ending.

mohsenkhanpour · 2018-07-27T16:14:09Z

@enyaxu This is the markdown file that I used. You can find more files in the same repository.

Should we need to change this to left or remain it?

The scroll bar is already at the left side. And it is fine.

I pull a request for source code viewer RTL support. please test, Thanks.

Sure.

mohsenkhanpour · 2018-07-27T16:50:28Z

@enyaxu I tested the pull request. Here is what I found:

Source code view still in ltr.
The menu option still applies to the next document rather than the active document.
Improved readability due to added margins.

enyaxu · 2018-07-28T15:14:36Z

@mohsenkhanpour
Thank you for feedback. I'll fixed asap.

enyaxu · 2018-08-01T15:08:08Z

@mohsenkhanpour @Jocs
First, I reimplement all RTL functions.

Source code view still in ltr.

The menu option still applies to the next document rather than the active document.

Above bugs are fixed.

Improved readability due to added margins.

For this, you can see my upload image, I think don't need to added margins. I tested from mac.

Also, I found Travis CI check failed, I check error like below.

appimage.AppImageConfiguration.FileAssociations: []appimage.FileAssociation: appimage.FileAssociation.Ext: ReadString: expects " or n, but found [, error found in #10 byte of ...|:[{"ext":["md","mark|..., bigger context ...|on.png","size":1025}],"fileAssociations":[{"ext":["md","markdown","mmd","mdown","mdtxt","mdtext"],"r|...

@Jocs Seems ext config for macosx has some error.

Thanks for review and testing.

mohsenkhanpour · 2018-08-01T16:55:27Z

Beautiful. I will pull and test as soon as I can. Thanks for your time and effort.

mohsenkhanpour · 2018-08-01T21:15:51Z

@enyaxu @Jocs I tested the new version:
The direction changes instantly.
Source code viewer can be changed to RTL.

I can confirm it is working flawlessly on Windows.

fxha · 2018-08-05T14:45:16Z

@enyaxu If you change the direction from LTR to RTL and back the direction attribute has conflicting values and stays RTL.

enyaxu · 2018-08-07T12:12:03Z

@fxha The el-dialog direction value is manual set by default to ltr, it will not changed with text direction in editor.

fxha · 2018-08-07T17:22:12Z

@enyaxu OK then the problem lies somewhere else because you cannot change the direction after setting it to RTL at runtime.

Jocs · 2018-08-12T08:53:57Z

@enyaxu Sorry, this PR has dragged on for so long. One reason is that I don't know much about RTL input. Another reason is that I am really busy recently. I just reviewed the code and ran it locally, with some questions.

is header tag right？
@mohsenkhanpour Can you help with this PR, And under the full test, is it running normally?

mohsenkhanpour · 2018-08-12T10:13:49Z

@Jocs I have been using this PR for some days and I have also given a copy to one of my friends who writes rtl text for his Jekyll blog.
Regarding the direction and style I haven't encountered any bugs so far.

What's that header tag? I haven't noticed it in my texts?

Jocs · 2018-08-12T12:43:30Z

@mohsenkhanpour If you cursor is in the active header paragraph, and the header tag will be shown on the left (ltr).

mohsenkhanpour · 2018-08-12T12:52:18Z

@Jocs It looks all good.

If there is anything specific that you want me to test I can do that.

Jocs · 2018-08-12T12:55:24Z

@mohsenkhanpour @enyaxu @fxha thank you all for this new feature, 👍

* chore(deps): bump dompurify to ^3.4.9 across desktop/muya/muyajs Updates the HTML sanitizer from ^3.4.3/^3.4.5 to ^3.4.9 (resolves to 3.4.11), deduping the two installed versions (3.4.3 + 3.4.7) into one. Clears Dependabot alerts #443/#451/#452/#453/#454/#455/#456. MarkText calls DOMPurify.sanitize() only with string input and RETURN_TRUSTED_TYPE: false (no IN_PLACE/RETURN_DOM/addHook/ SAFE_FOR_TEMPLATES), so none of these CVEs were reachable; the bump is defense-in-depth for the editor's HTML sanitization path. DOMPurify 3.4.8+ hardened cross-realm namespace validation, which the happy-dom test environment does not satisfy: under happy-dom it strips every element, even default-allowed tags like <p>/<h1>. Real Chromium/Electron is unaffected (verified: jsdom, which matches production DOM behavior, sanitizes correctly). Move the five DOMPurify-dependent muya specs to the jsdom environment and declare jsdom as a muya devDependency. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * chore(deps): bump happy-dom to ^20.8.9 in muya Updates muya's vitest DOM test environment from ^15.11.7 to ^20.8.9 (resolves to 20.10.6). Clears Dependabot alerts #426/#427/#428/#434/#437/#438 (2 critical "VM context escape / RCE", 4 high). happy-dom is a devDependency used only as the unit-test DOM; it is never bundled into the shipped app, and the muya suites run trusted fixtures, so these CVEs were not reachable. The bump keeps the test toolchain current and clears the critical badges. The full muya unit suite (143 files) passes on 20.x — the DOMPurify-dependent specs already moved to jsdom in the previous commit. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * chore(deps): update vite to patched 7.3.5 / 8.0.16 Bumps vite to the patched releases across the workspace (desktop ^7.3.5, muya + muya-e2e ^8.0.16), updating the declared floors so installs can't regress below the fix. Clears Dependabot alerts #441/#442/#447/#448 (server.fs.deny bypass + launch-editor NTLMv2 disclosure — both Windows dev-server only; vite is build tooling, never shipped to users). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * chore(deps): pin transitive deps to patched versions via pnpm overrides Adds range-scoped pnpm.overrides to force patched releases of the transitive dependencies Dependabot flagged, while leaving unaffected older majors in place (undici 6.x, esbuild 0.25.x, js-yaml 3.x): form-data 4.0.5 -> 4.0.6 (#450 CRLF injection) tmp 0.2.5 -> 0.2.6 (#411 path traversal) tar 7.5.15 -> 7.5.16 (#449 PAX file smuggling) ws 8.20.1 -> 8.21.0 (#444 memory-exhaustion DoS) undici 7.24/7.25 -> 7.28.0 (#457 SOCKS5 TLS bypass; keeps 6.25.0) esbuild 0.27/0.28.0-> 0.28.1 (#439 dev-server file read; keeps 0.25.x) @babel/core 7.29.0 -> 7.29.6 (#445 sourceMappingURL file read) js-yaml 4.1.1 -> 4.2.0 (#446 merge-key DoS; keeps 3.14.2) All are build/test/website tooling reachable only on developer/CI machines, never bundled into the shipped app. js-yaml 3.14.2 remains via the website's gray-matter (no 3.x patch exists); it parses only trusted first-party content, so #446 is not exploitable there and will be dismissed on GitHub. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

* feature: Add experiment RTL support * fix: binding to currentfile textdirection * feature: add sourcecode RTL support * feature: add text direction menu upgrade * fix sourceCode does't change from menu switch text direction

Jocs requested review from Jocs and fxha July 25, 2018 07:52

fxha reviewed Jul 25, 2018

View reviewed changes

enyaxu force-pushed the feature/421 branch from c96af53 to 95c8ee2 Compare July 26, 2018 13:09

enyaxu added 5 commits August 1, 2018 00:27

feature: Add experiment RTL support

3065400

fix: binding to currentfile textdirection

570c0a6

feature: add sourcecode RTL support

267e384

feature: add text direction menu upgrade

7ae28b4

fix sourceCode does't change from menu switch text direction

e005750

enyaxu force-pushed the feature/421 branch from 4718eca to e005750 Compare August 1, 2018 14:44

fxha approved these changes Aug 9, 2018

View reviewed changes

Jocs approved these changes Aug 12, 2018

View reviewed changes

Jocs merged commit c01c65c into marktext:master Aug 12, 2018

Jocs mentioned this pull request Jun 20, 2026

chore(security): remediate 25 Dependabot alerts #4596

Merged

Conversation

enyaxu commented Jul 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

fxha left a comment

Choose a reason for hiding this comment

Uh oh!

fxha Jul 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enyaxu Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fxha Jul 26, 2018

Choose a reason for hiding this comment

Uh oh!

mohsenkhanpour Jul 26, 2018

Choose a reason for hiding this comment

Uh oh!

Jocs commented Jul 26, 2018

Uh oh!

enyaxu commented Jul 26, 2018 via email

Uh oh!

mohsenkhanpour commented Jul 26, 2018

Uh oh!

Jocs commented Jul 26, 2018

Uh oh!

fxha commented Jul 26, 2018

Uh oh!

mohsenkhanpour commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jocs commented Jul 26, 2018

Uh oh!

enyaxu commented Jul 26, 2018

Uh oh!

enyaxu commented Jul 26, 2018

Uh oh!

enyaxu commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

enyaxu commented Jul 26, 2018

Uh oh!

Jocs commented Jul 27, 2018

Uh oh!

Jocs commented Jul 27, 2018

Uh oh!

mohsenkhanpour commented Jul 27, 2018

Uh oh!

mohsenkhanpour commented Jul 27, 2018

Uh oh!

enyaxu commented Jul 28, 2018

Uh oh!

enyaxu commented Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohsenkhanpour commented Aug 1, 2018

Uh oh!

mohsenkhanpour commented Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fxha commented Aug 5, 2018

Uh oh!

enyaxu commented Aug 7, 2018

Uh oh!

fxha commented Aug 7, 2018

Uh oh!

Jocs commented Aug 12, 2018

Uh oh!

mohsenkhanpour commented Aug 12, 2018

Uh oh!

Jocs commented Aug 12, 2018

Uh oh!

mohsenkhanpour commented Aug 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jocs commented Aug 12, 2018

enyaxu commented Jul 25, 2018 •

edited

Loading

fxha Jul 25, 2018 •

edited

Loading

enyaxu Jul 26, 2018 •

edited

Loading

mohsenkhanpour commented Jul 26, 2018 •

edited

Loading

enyaxu commented Jul 26, 2018 •

edited

Loading

enyaxu commented Aug 1, 2018 •

edited

Loading

mohsenkhanpour commented Aug 1, 2018 •

edited

Loading

mohsenkhanpour commented Aug 12, 2018 •

edited

Loading