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

Fix reversed the statement when mixed RTL with LTR #169459

Closed
AhmedElTabarani opened this issue Dec 17, 2022 · 4 comments
Closed

Fix reversed the statement when mixed RTL with LTR #169459

AhmedElTabarani opened this issue Dec 17, 2022 · 4 comments
Assignees
Labels
editor-RTL Editor Right-To-Left or Bi-Di issues info-needed Issue requires more information from poster

Comments

@AhmedElTabarani
Copy link

AhmedElTabarani commented Dec 17, 2022

Does this issue occur when all extensions are disabled?: Yes/No

  • VS Code Version: 1.74.1
  • OS Version: Windows_NT x64 10.0.19044

Statements reversed when mixed RTL with LTR

Steps to Reproduce:

Actually: right order should be as the numbers, should order the words from right to left
208216659-1a055824-92c1-4687-9944-011e1c503d53

Expected: as the order of the numbers

image

Text:

// ما هو JWT ؟
// أولًا هو اختصار لـ Json Web Token
// والـ Json هو الشكل المتعارف عليه في تخزين البيانات

Note: GitHub displayed the text correctly and also make the whole text go to right of the page
this issue is not "all the text not going to the right of the screen", because that's another issue
this problem is with "sentence imbalance in general"

@AhmedElTabarani AhmedElTabarani changed the title Fix reversed the statement with mix RTL with LTR Fix reversed the statement when mixed RTL with LTR Dec 17, 2022
@alexdima
Copy link
Member

alexdima commented Dec 19, 2022

@AhmedElTabarani Is there any code editor which is able to display the text in the desired order (as you post in the second screenshot)? This is the HTML we generate on our side in order to render the first line:

<span dir="ltr"><span style="unicode-bidi:isolate">//&nbsp;ما&nbsp;هو&nbsp;JWT&nbsp;؟</span></span>

I've created a fiddle with it at https://jsfiddle.net/Lnsfy1wc/

image

How should the HTML we generate look like for the first line in order to achieve the desired order and what would be the rules behind generating it in that way?

@alexdima alexdima added info-needed Issue requires more information from poster editor-RTL Editor Right-To-Left or Bi-Di issues labels Dec 19, 2022
@AhmedElTabarani
Copy link
Author

AhmedElTabarani commented Dec 21, 2022

Is there any code editor which is able to display the text in the desired order

I didn't see any code editor support that, any way most of the programs support RTL as GitHub did.

How should the HTML we generate look like

in my website I use two way to achieve that

unicode-bidi: "plaintext"

or

<span dir="auto">...</span>

preview:

image

Although // go to the right, but this will nice improvement and much readable right now

@AhmedElTabarani
Copy link
Author

I think it is better to make an option in View to make the editor render each line as dir= rtl or ltr or auto to avoid any side effects and to be tested better

@alexdima
Copy link
Member

@AhmedElTabarani Thank you for the two suggestions. I tried both and then did a test pass over various bidi samples I collected over the years. From my testing, I don't think we can ship any of these changes without breaking other samples.

To get the rendering that you request, we will need to implement the bidi algorithm in JavaScript. We currently delegate the text rendering in the editor to the browser. I suggest we continue tracking this issue in #11770 . Please see below what I tried:

using dir="auto"

Running with the change:

diff --git a/src/vs/editor/common/viewLayout/viewLineRenderer.ts b/src/vs/editor/common/viewLayout/viewLineRenderer.ts
index 0e94382f9a8..f4f31951fc8 100644
--- a/src/vs/editor/common/viewLayout/viewLineRenderer.ts
+++ b/src/vs/editor/common/viewLayout/viewLineRenderer.ts
@@ -939,7 +939,7 @@ function _renderLine(input: ResolvedRenderLineInput, sb: StringBuilder): RenderL
 	let partDisplacement = 0;
 
 	if (containsRTL) {
-		sb.appendString('<span dir="ltr">');
+		sb.appendString('<span dir="auto">');
 	} else {
 		sb.appendString('<span>');
 	}
Sample
{
  "invoice_vip":[
    ["🖨️ چاپ فاکتور","🎨 تنظیمات"]
  ]
}
current rendering image
with dir="auto" image

using unicode-bidi:plaintext

Running with the change:

diff --git a/src/vs/editor/common/viewLayout/viewLineRenderer.ts b/src/vs/editor/common/viewLayout/viewLineRenderer.ts
index 0e94382f9a8..a9aa11b6872 100644
--- a/src/vs/editor/common/viewLayout/viewLineRenderer.ts
+++ b/src/vs/editor/common/viewLayout/viewLineRenderer.ts
@@ -957,7 +957,7 @@ function _renderLine(input: ResolvedRenderLineInput, sb: StringBuilder): RenderL
 
 		sb.appendString('<span ');
 		if (partContainsRTL) {
-			sb.appendString('style="unicode-bidi:isolate" ');
+			sb.appendString('style="unicode-bidi:plaintext" ');
 		}
 		sb.appendString('class="');
 		sb.appendString(partRendersWhitespaceWithWidth ? 'mtkz' : partType);
Sample
<b>
	9- هزینه کاهش ارزش دریافتنی‌ها    
</b>
current rendering image
unicode-bidi:plaintext image

@alexdima alexdima closed this as not planned Won't fix, can't repro, duplicate, stale Dec 21, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-RTL Editor Right-To-Left or Bi-Di issues info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

3 participants