Opened 6 years ago

Closed 6 years ago

#11641 closed Bug (fixed)

Switching between modes in framed editor removes content styles for inline editor

Reported by: Piotrek Koszuliński Owned by: Piotr Jasiun
Priority: Normal Milestone: CKEditor 4.3.4
Component: General Version:
Keywords: Cc:

Description (last modified by Piotrek Koszuliński)

  1. Download 11641.html
  2. Switch between modes in first editor.
  3. See that content in inline editor lost styles.

Attachments (2)

11641.html (4.8 KB) - added by Piotrek Koszuliński 6 years ago.
11641.webm (505.3 KB) - added by Piotrek Koszuliński 6 years ago.

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by Piotrek Koszuliński

Attachment: 11641.html added

comment:1 Changed 6 years ago by Piotrek Koszuliński

Description: modified (diff)
Milestone: CKEditor 4.3.4
Status: newconfirmed

Changed 6 years ago by Piotrek Koszuliński

Attachment: 11641.webm added

comment:2 Changed 6 years ago by Olek Nowodziński

cc: love this one!

comment:3 Changed 6 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedassigned

comment:4 Changed 6 years ago by Piotr Jasiun

Status: assignedreview

The problem was in editable.detach method. When editor was in textarea mode this method removes styles from the page document (this.getDocument() for textarea was page document).

Changes in t/11641 and corresponding test branch.

comment:5 Changed 6 years ago by Piotrek Koszuliński

I wonder if there's any more straightforward way of checking if stylesheet should be removed. What if someone used other source mode than sourcearea? For example source code editor?

What if we set editable.hasStylesheet property when it adds stylesheet and then check it in detach?

PS. There are 3 minor code style issues in tests.

comment:6 Changed 6 years ago by Piotr Jasiun

I'm also not sure if this.is( 'textarea' ) is the best way to check this, but this way is already used in other places in this file (here and here) and I believe that it is most important to use the same check everywhere. If we want to let user use custom source code editor we should rethink all of the if( this.is( 'textarea' ) ) conditions.

comment:7 Changed 6 years ago by Piotrek Koszuliński

Status: reviewreview_passed

Ok, you're right. We already use this check, so in this case we can do that too. However, we should take the opportunity to fix this as soon as we'll make some refactoring of editable class. The entire addCss logic is tangled. There are better ways to ensure that stylesheet will be preserved as long as there's at least one editor using it.

comment:8 Changed 6 years ago by Piotr Jasiun

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