Opened 6 years ago

Closed 5 years ago

#3576 closed Bug (fixed)

Style system incorrectly remove element

Reported by: garry.yao Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.3
Component: Core : Styles Version: SVN (CKEditor) - OLD
Keywords: Confirmed Review+ Cc:

Description

If you're applying any 'span' based style on the following range:

<p>^this <span>tag</span> should not be removed.^</p>

Then the span will always be removed, which is wrong.

Attachments (3)

3576.patch (8.9 KB) - added by garry.yao 6 years ago.
3576_2.patch (1.8 KB) - added by garry.yao 5 years ago.
3576_3.patch (1.9 KB) - added by garry.yao 5 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 6 years ago by garry.yao

  • Keywords Review? added
  • Status changed from new to assigned

Beside the fix, the patch also align the styles TS to the current trunk.

Changed 6 years ago by garry.yao

comment:2 Changed 6 years ago by fredck

  • Milestone changed from CKEditor 3.0 to CKEditor 3.1
  • Priority changed from High to Normal

This doesn't look like a big issue really. We have the same behavior in V2 and we have never had complains about it.

I'm postponing this ticket so we can still think if the change is valid. If we have few code changes for it, we may even consider this.

comment:3 Changed 6 years ago by tobiasz.cudnik

  • Keywords Review- added; Review? removed

I'm still getting this issue on FF 3.5 with this result:

<p>
	<span style="background-color: rgb(255, 215, 0);">this tag should not be removed.</span></p>

comment:4 Changed 6 years ago by garry.yao

Defer this fixing since at least span with any style/attribute won't be affected.

comment:5 Changed 6 years ago by garry.yao

  • Owner garry.yao deleted
  • Status changed from assigned to new

comment:6 Changed 6 years ago by garry.yao

  • Milestone changed from CKEditor 3.1 to CKEditor 3.2
  • Version set to SVN (CKEditor)

comment:7 Changed 6 years ago by fredck

  • Milestone changed from CKEditor 3.2 to CKEditor 3.3

comment:8 Changed 5 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from new to assigned

Changed 5 years ago by garry.yao

comment:9 Changed 5 years ago by garry.yao

  • Component changed from General to Core : Styles
  • Keywords Review? added; Review- removed

Align previous patch with trunk.

comment:10 Changed 5 years ago by alfonsoml

  • Keywords Review- added; Review? removed

Both Firefox 3.6 and IE8 remove the span in the testcase.

Changed 5 years ago by garry.yao

comment:11 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

Adding the missed checks for 'removeEmpty', I wonder why browsers don't implement the return value of 'removeAttribute' :(

comment:12 Changed 5 years ago by alfonsoml

  • Keywords Review+ added; Review? removed

comment:13 Changed 5 years ago by garry.yao

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed with [5260].

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