Opened 14 years ago
Closed 14 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 14 years ago by
Owner: | set to Tobiasz Cudnik |
---|---|
Status: | new → assigned |
comment:2 Changed 14 years ago by
Changed 14 years ago by
Attachment: | 5910.patch added |
---|
comment:3 Changed 14 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 14 years ago by
Keywords: | Review- added; Review? removed |
---|
KISS, switching the margins in the bidi plugin directly.
Changed 14 years ago by
Attachment: | 5910_2.patch added |
---|
comment:5 Changed 14 years ago by
Keywords: | Review? added; Review- removed |
---|
This patch should include all features, along with better indentation from multiple selection.
comment:6 Changed 14 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 14 years ago by
Attachment: | 5910_3.patch added |
---|
comment:7 Changed 14 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 14 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 14 years ago by
Attachment: | 5910_4.patch added |
---|
comment:9 Changed 14 years ago by
Owner: | changed from Tobiasz Cudnik to Frederico Caldeira Knabben |
---|---|
Status: | review_failed → review |
comment:10 Changed 14 years ago by
Status: | review → review_passed |
---|
comment:11 Changed 14 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.