Opened 8 years ago

Closed 7 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 8 years ago.
3576_2.patch (1.8 KB) - added by Garry Yao 7 years ago.
3576_3.patch (1.9 KB) - added by Garry Yao 7 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 8 years ago by Garry Yao

Keywords: Review? added
Status: newassigned

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

Changed 8 years ago by Garry Yao

Attachment: 3576.patch added

comment:2 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.0CKEditor 3.1
Priority: HighNormal

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 8 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 8 years ago by Garry Yao

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

comment:5 Changed 8 years ago by Garry Yao

Owner: Garry Yao deleted
Status: assignednew

comment:6 Changed 8 years ago by Garry Yao

Milestone: CKEditor 3.1CKEditor 3.2
Version: SVN (CKEditor)

comment:7 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.2CKEditor 3.3

comment:8 Changed 7 years ago by Garry Yao

Owner: set to Garry Yao
Status: newassigned

Changed 7 years ago by Garry Yao

Attachment: 3576_2.patch added

comment:9 Changed 7 years ago by Garry Yao

Component: GeneralCore : Styles
Keywords: Review? added; Review- removed

Align previous patch with trunk.

comment:10 Changed 7 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review- added; Review? removed

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

Changed 7 years ago by Garry Yao

Attachment: 3576_3.patch added

comment:11 Changed 7 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 7 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review+ added; Review? removed

comment:13 Changed 7 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [5260].

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