Ticket #3576 (closed Bug: fixed)

Opened 6 years ago

Last modified 5 years ago

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

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.

Change History

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

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

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 5 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 5 years ago by garry.yao

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

comment:5 Changed 5 years ago by garry.yao

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

comment:6 Changed 5 years ago by garry.yao

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

comment:7 Changed 5 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

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

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

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

Fixed with [5260].

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