Opened 15 years ago
Closed 15 years ago
#5910 closed Bug (fixed)
BIDI: Indentation should work with mixed BIDI documents
| Reported by: | Damian | Owned by: | Frederico Caldeira Knabben |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 3.4 |
| Component: | General | Version: | |
| Keywords: | IBM | Cc: | Satya Minnekanti, joek |
Description
Direction sensitive operations like indentation and alignment should work properly according to the base language direction of the relevant part of the document. The toolbar icons for these functions should also render correctly when moving between RTL and LTR content.
Example problem 1
Set this content in source and switch back to rich text.
<p dir="rtl">RTL content</p>
Notice that it isn't possible to use the indentation buttons in the editor (left margin is applied instead of right).
Example problem 2
Using the same content as above, notice that the justify icons shows left aligned, when it should show right aligned.
Attachments (4)
Change History (15)
comment:1 Changed 15 years ago by
| Owner: | set to Tobiasz Cudnik |
|---|---|
| Status: | new → assigned |
comment:2 Changed 15 years ago by
Changed 15 years ago by
| Attachment: | 5910.patch added |
|---|
comment:3 Changed 15 years ago by
| Keywords: | Review? added |
|---|
Maybe we need some event about switching the direction to notify other plugins (like this one) ? Or maybe we will implement margin flipping feature (and possibly others) of inner content, inside plugin which switched the direction ?
comment:4 Changed 15 years ago by
| Keywords: | Review- added; Review? removed |
|---|
KISS, switching the margins in the bidi plugin directly.
Changed 15 years ago by
| Attachment: | 5910_2.patch added |
|---|
comment:5 Changed 15 years ago by
| Keywords: | Review? added; Review- removed |
|---|
This patch should include all features, along with better indentation from multiple selection.
comment:6 Changed 15 years ago by
| Keywords: | Review- added; Review? removed |
|---|
Things are pretty broken after the patch:
- The indentation button stopped working.
- The direction buttons get stuck once applied. It's not possible to switch to another direction. We must first remove the applied direction and then set the other.
Changed 15 years ago by
| Attachment: | 5910_3.patch added |
|---|
comment:7 Changed 15 years ago by
| Keywords: | Review? added; Review- removed |
|---|
This is simplified patch using getComputedStyle instead of DOM traversing to determine text direction.
Additionally i've fixed dir switch buttons.
comment:8 Changed 15 years ago by
| Status: | review → review_failed |
|---|
- KISS, we can get rid of indentCssProperty and make things work properly. Insisting with it is a bad idea.
- getComputedStyle should be used in the bidi plugin also, instead of hacks to find out the inherited direction.
Changed 15 years ago by
| Attachment: | 5910_4.patch added |
|---|
comment:9 Changed 15 years ago by
| Owner: | changed from Tobiasz Cudnik to Frederico Caldeira Knabben |
|---|---|
| Status: | review_failed → review |
comment:10 Changed 15 years ago by
| Status: | review → review_passed |
|---|
comment:11 Changed 15 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Fixed with [5746].

Related to this, one o the feature of the BiDi tools is that, when switching the direction, the left and right margin styles will be also switched, effectively mirroring the block.