Opened 7 years ago

Closed 7 years ago

#6865 closed Bug (fixed)

BiDi buttons are not reflected when a change is done through a dialog

Reported by: Sa'ar Zac Elias Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.5.3
Component: Core : BiDi Version: 3.4
Keywords: Cc:

Description

  • Open a sample and create a div.
  • Note that the toolbar icon shows LTR.
  • Right click on the div, and choose edit div.
  • Go to the advanced tab and choose the RTL from the Language Direction dropdown.
  • Click OK.

Note that the toolbar icon shows LTR.

Attachments (5)

6865.patch (3.8 KB) - added by Garry Yao 7 years ago.
6865_2.patch (801 bytes) - added by Garry Yao 7 years ago.
6865_3.patch (6.0 KB) - added by Garry Yao 7 years ago.
6865_4.patch (4.3 KB) - added by Garry Yao 7 years ago.
6865_5.patch (4.7 KB) - added by Garry Yao 7 years ago.

Download all attachments as: .zip

Change History (20)

Changed 7 years ago by Garry Yao

Attachment: 6865.patch added

comment:1 Changed 7 years ago by Garry Yao

Owner: set to Garry Yao
Status: newreview

comment:2 Changed 7 years ago by Wiktor Walc

Milestone: CKEditor 3.5.2

comment:3 Changed 7 years ago by Sa'ar Zac Elias

Status: reviewreview_failed
  • There're missing fields now (language code & title) and we already have the css class field in the general tab.
  • It's impossible to edit class names of the div through the general tab (maybe becuase the same field appears on both tabs).
  • Note that the way we handle styles from the styles set is not treated.

Changed 7 years ago by Garry Yao

Attachment: 6865_2.patch added

comment:4 Changed 7 years ago by Garry Yao

Status: review_failedreview

This's just a temporary fix, we need to seek for a way of firing this event at element::setAttribute(s) level or even by "DOMAttrModified" event.

comment:5 Changed 7 years ago by Sa'ar Zac Elias

Status: reviewreview_failed

The div dialog was just an example, also affects other dialogs (e.g. link). Let's find a way to trigger the event on attribute modification in general.

comment:6 Changed 7 years ago by Sa'ar Zac Elias

Milestone: CKEditor 3.5.3

My bad, the link dialog is not a good example as the buttons only care about block level elements (maybe a point of thought?). All in all, this is an R- as the patch doesn't work for me, anyway we should still seek for a more dynamic way.

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

Milestone: CKEditor 3.5.2

comment:8 Changed 7 years ago by Sa'ar Zac Elias

Milestone: CKEditor 3.5.2CKEditor 3.5.3

Changed 7 years ago by Garry Yao

Attachment: 6865_3.patch added

comment:9 Changed 7 years ago by Garry Yao

Status: review_failedreview
Version: 3.4

Ok now a generic solution proposed.

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

Status: reviewreview_failed

Note that el.setAttribute( 'style', 'sthg' ) is not the same as el.setStyle( 'sthg', 'sthg' ), so we can't map setAttribute to setStyle in that sense.

Changed 7 years ago by Garry Yao

Attachment: 6865_4.patch added

comment:11 Changed 7 years ago by Garry Yao

Status: review_failedreview

I'd not go with style parsing approach to NOT bring a performance impacts.

comment:12 Changed 7 years ago by Sa'ar Zac Elias

Status: reviewreview_failed
  • Trying to insert a table results in a js error.
  • We should also consider removeAttribute and removeStyle.

Changed 7 years ago by Garry Yao

Attachment: 6865_5.patch added

comment:13 Changed 7 years ago by Garry Yao

Status: review_failedreview

Trying to insert a table results in a js error.

Cannot reproduce this issue.

TC added with [6474].

comment:14 Changed 7 years ago by Sa'ar Zac Elias

Status: reviewreview_passed

comment:15 Changed 7 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [6478].

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