Opened 3 years ago

Closed 3 years ago

#13128 closed Bug (fixed)

ID attributes removed on drag and drop and copy and paste

Reported by: Piotr Jasiun Owned by: Olek Nowodziński
Priority: Must have (possibly next milestone) Milestone: CKEditor 4.5.0
Component: General Version: 4.5.0 Beta
Keywords: Cc:

Description

After changes introduced in #13042 id attributes are removed on drag and drop and copy and paste block widgets. They seems to be generally removed during these operations and they should not, so anchors may work fine.

This is most probably because getHtmlFromRange and extractHtmlFromRange do this.

Change History (10)

comment:1 Changed 3 years ago by Piotr Jasiun

I restored IDs in tests removed in #13042 and pushed it to t/13128.

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

Milestone: CKEditor 4.5.0
Status: newconfirmed

comment:3 Changed 3 years ago by Piotrek Koszuliński

Priority: NormalHigh

comment:4 Changed 3 years ago by Olek Nowodziński

Owner: set to Olek Nowodziński
Status: confirmedassigned

comment:5 Changed 3 years ago by Olek Nowodziński

Status: assignedreview

branch:t/13128

It was too easy. But worth a shot.

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

One question - did you consciously avoid cloning ids of a partially selected elements? Did you compare this behaviour with the native cloneContents()?

Last edited 3 years ago by Piotrek Koszuliński (previous) (diff)

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

Status: reviewreview_failed

To make sure that one day I'll get an answer to the above question - R- :P

comment:8 in reply to:  6 Changed 3 years ago by Olek Nowodziński

Status: review_failedassigned

Replying to Reinmar:

One question - did you consciously avoid cloning ids of a partially selected elements? Did you compare this behaviour with the native cloneContents()?

Nice catch.

For the following ranges

<p>[xxx<b id="y">abc</b>zzz]</p>
<p>xxx<b id="y">ab[c</b>zzz]</p>

native range.cloneContents() returns the following docFragments in Chrome, FF and IE11

xxx<b id="y">abc</b>zzz
<b id="y">c</b>zzz

which means that ids of partially selected elements are preserved. And this is the right thing to do, I believe.

I pushed a test for such a case to branch:t/13128. Focusing on the fix at the moment.

comment:9 Changed 3 years ago by Olek Nowodziński

Status: assignedreview

Let's see what it's worth: branch:t/13128d.

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

Resolution: fixed
Status: reviewclosed

I made some small docs and tests improvements and merged the branch to major with git:0514f02.

Note - do not alter too many existing tests in cases like this one without adding a separate TC for the issue. For example, in range/* the changes were ok, because you changed existing tests and added new ones. But in widgets you only modified existing tests. So it may be unclear in the future that this small ids were meant to check something.

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