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

Check RTL support #3788

Closed
Reinmar opened this issue Jul 22, 2016 · 10 comments
Closed

Check RTL support #3788

Reinmar opened this issue Jul 22, 2016 · 10 comments
Assignees
Labels
package:engine type:task This issue reports a chore (non-production change) and other types of "todos".
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Jul 22, 2016

We should check how engine and existing features work with RTL content.

@Reinmar Reinmar self-assigned this Sep 23, 2016
@Reinmar
Copy link
Member Author

Reinmar commented Sep 23, 2016

  1. Open any sample.
  2. Change language to Arabic.
  3. Press 'a' multiple times.
  4. Shift+left.

screen shot 2016-09-23 at 12 19 37

As you can see the range boundaries are reversed when printed on the console. It must be checked whether they really are reversed in the DOM.

But I'm afraid they are, because if you press Ctrl+B, then:

screen shot 2016-09-23 at 12 21 19

@Reinmar
Copy link
Member Author

Reinmar commented Sep 23, 2016

Note: The above can only be reproduced in Chrome and Safari. In Firefox you get:

screen shot 2016-09-23 at 12 31 41

However, when I copied the console output here, I got:

<heading1>شششش[]</heading1>
<heading1>ششش[ش]</heading1>
<heading1>شششش[]</heading1>
<heading1>ششش[<$text bold="true">ش</$text>]</heading1>

@Reinmar
Copy link
Member Author

Reinmar commented Sep 23, 2016

screen shot 2016-09-23 at 12 50 21

Contiguous DOM selection, non-contiguous box.

@fredck
Copy link
Contributor

fredck commented Sep 26, 2016

A good way to test it is comparing the behavior we have in our implementation with the one in the native contenteditable.

@Reinmar
Copy link
Member Author

Reinmar commented Oct 7, 2016

This is weird:
oct-07-2016 15-46-28

@Reinmar
Copy link
Member Author

Reinmar commented Oct 7, 2016

OK, it works as expected if the editable has dir=rtl. Previously, I wrapped the whole editor in div[dir=rtl] but, apparently, it must be the editable which has the attr.

@fredck
Copy link
Contributor

fredck commented Oct 7, 2016

Remember that we must, additionally, checked "mixed direction" in the content, like an Arabic word or phrase inside English content. Such content may appear within a <span dir="rtl"> element but also without any element around it.

@Reinmar
Copy link
Member Author

Reinmar commented Oct 7, 2016

I created a manual test in which you can compare, and eventually debug, RTL support issues.

The most important thing I understood is that if you have a mixed content (RTL text next to LTR or RTL text in LTR container), then really weird things happen. Fortunately, it's exactly the same behaviour which you get in native contenteditable. Browsers also tend to differ between themselves in such cases.

So, I consider this done and everything is working fine (tested in Chrome and Firefox).

Tests were added in ckeditor/ckeditor5-typing@64ad8c1

@Reinmar Reinmar closed this as completed Oct 7, 2016
@Reinmar
Copy link
Member Author

Reinmar commented Oct 7, 2016

Remember that we must, additionally, checked "mixed direction" in the content, like an Arabic word or phrase inside English content. Such content may appear within a element but also without any element around it.

I tested mixed content without those spans. I don't think that something could be wrong with spans, because it's browser who renders that mess and moves the selection. In the DOM everything is in the right order – range end is always after range start in DOM order. E.g. this works nice, even though visually the range is discontiguous:

oct-07-2016 17-15-33

@aliqasemzadeh
Copy link

Hello,
I think having RTL support is so easy.
Just don't add dir=ltr when replace editor.

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 4 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:task This issue reports a chore (non-production change) and other types of "todos". package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

No branches or pull requests

4 participants