Opened 4 years ago

Closed 4 years ago

#12135 closed Bug (fixed)

Remove format not working on widgets

Reported by: Tsvetelin Novkirishki Owned by: Marek Lewandowski
Priority: Normal Milestone: CKEditor 4.4.4
Component: UI : Widgets Version: 4.3 Beta
Keywords: Cc:

Description

Descriptive summary: When there are styles applied to a widget, Remove format does nothing.

Steps to reproduce:

  1. Insert some widget
  2. Bold it
  3. Click on Remove format button and nothing happens

Browser and OS: Chrome and Firefox under both Windows and Linux

Change History (10)

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

Status: newconfirmed
Version: 4.4.3 (GitHub - master)4.3 Beta

Confirmed. Remove format seems to lose widget selection.

comment:2 Changed 4 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.4.3

comment:3 Changed 4 years ago by Marek Lewandowski

Owner: set to Marek Lewandowski
Status: confirmedassigned

comment:4 Changed 4 years ago by Marek Lewandowski

Status: assignedreview

Pushed to t/12135 at dev.

I think it would be pretty good to add also editor seleciton assertion after first assertion in test remove format outside the contenteditable=false:

assert.areEqual( '<p>[<span contenteditable="false">foo</span>]</p>', bender.tools.selection.getWithHtml( bot.editor ), 'editor seleciton' );

But <div data-cke-hidden-sel="1" data-cke-temp="1" style="position:fixed;top:0;left:-1000px">&nbsp;</div> is being added, any idea how to assert content gracefully in this case?

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

Hm... We can add this case to tools.html.compareInnerHtml, so it strips elements with data-cke-temp attributes. I've got to work on this function anyway, so I'll do that before reviewing. Leave this to me.

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

Milestone: CKEditor 4.4.3CKEditor 4.4.4

The 4.4.3 milestone was shortened, so the remaining tickets must be postponed.

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

In https://github.com/benderjs/benderjs-ckeditor/issues/7 and https://github.com/benderjs/benderjs-ckeditor/issues/8 I improved tools, so I'm able to resolve problem mentioned in comment:4 finally.

comment:8 Changed 4 years ago by Piotrek Koszuliński

Status: reviewreview_failed
  1. Please split tests into multiple cases (functions).
  2. Split that one commit into a code style fixes and bug fix, because it's hard to understand what really is the fix and why it works.

comment:9 Changed 4 years ago by Marek Lewandowski

Status: review_failedreview

Changed branch history.

Pushed to t/12135 at dev.

comment:10 Changed 4 years ago by Piotrek Koszuliński

Resolution: fixed
Status: reviewclosed

Corrected:

  • Nested tags in tests (strong>strong)
  • Selections finishing in non-editable locations - we do not allow them.
  • The getNext's callback name.

Merged to master on git:8cabc81.

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