Opened 10 years ago

Closed 10 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)

5910.patch (4.8 KB) - added by Tobiasz Cudnik 10 years ago.
5910_2.patch (5.7 KB) - added by Tobiasz Cudnik 10 years ago.
5910_3.patch (4.4 KB) - added by Tobiasz Cudnik 10 years ago.
5910_4.patch (3.8 KB) - added by Frederico Caldeira Knabben 10 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 10 years ago by Tobiasz Cudnik

Owner: set to Tobiasz Cudnik
Status: newassigned

comment:2 Changed 10 years ago by Frederico Caldeira Knabben

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.

Changed 10 years ago by Tobiasz Cudnik

Attachment: 5910.patch added

comment:3 Changed 10 years ago by Tobiasz Cudnik

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 10 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

KISS, switching the margins in the bidi plugin directly.

Changed 10 years ago by Tobiasz Cudnik

Attachment: 5910_2.patch added

comment:5 Changed 10 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

This patch should include all features, along with better indentation from multiple selection.

comment:6 Changed 10 years ago by Frederico Caldeira Knabben

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 10 years ago by Tobiasz Cudnik

Attachment: 5910_3.patch added

comment:7 Changed 10 years ago by Tobiasz Cudnik

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 10 years ago by Frederico Caldeira Knabben

Status: reviewreview_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 10 years ago by Frederico Caldeira Knabben

Attachment: 5910_4.patch added

comment:9 Changed 10 years ago by Frederico Caldeira Knabben

Owner: changed from Tobiasz Cudnik to Frederico Caldeira Knabben
Status: review_failedreview

comment:10 Changed 10 years ago by Sa'ar Zac Elias

Status: reviewreview_passed

comment:11 Changed 10 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: review_passedclosed

Fixed with [5746].

Note: See TracTickets for help on using tickets.
© 2003 – 2020 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy