Opened 10 years ago
Closed 10 years ago
#13449 closed Bug (fixed)
Cut reverse notes
Reported by: | Piotr Jasiun | Owned by: | Szymon Cofalik |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.5.0 |
Component: | General | Version: | 4.5.0 Beta |
Keywords: | Cc: |
Description
- Open:
samples/old/replacebycode.html
- Select from the begging to 'Americans' (
[<h1>Apollo 11<h1><p>Apollo 11 was the spaceflight that landed the first humans,]
) - Press Ctrl+X.
Result:
- the <p> is changed to <h1> (what is not that bad)
- all nodes in the paragraph are in the REVERS order (what is the biggest WTF of the week)
So we have:
, on the Moon on July 20, 1969, at 20:18 UTC. Armstrong became the first to step onto the lunar surface 6 hours later on July 21 at 02:56 UTC.Buzz Aldrin and Neil Armstrong Americans
instead of:
, Americans Neil Armstrong and Buzz Aldrin, on the Moon on July 20, 1969, at 20:18 UTC. Armstrong became the first to step onto the lunar surface 6 hours later on July 21 at 02:56 UTC.
Reproduced on Opera, Chrome and Firefox, so wherever we have custom "Cut" handling.
Change History (9)
comment:1 Changed 10 years ago by
Owner: | set to Szymon Cofalik |
---|---|
Status: | new → assigned |
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
Simpified case:
- Open editor with following content and selection:
<div>[Apollo 11</div> <p><strong>foo</strong> bar <a href="http://en.wikipedia.org/wiki/Neil_Armstrong">baz</a> ]ban <a href="http://en.wikipedia.org/wiki/Buzz_Aldrin">buz</a></p>
- Press Ctrl + X
Result:
<p><a href="http://en.wikipedia.org/wiki/Buzz_Aldrin">buz</a>ban </p>
Expected result:
<p>ban <a href="http://en.wikipedia.org/wiki/Buzz_Aldrin">buz</a></p>
comment:4 Changed 10 years ago by
Fix available on branch:t/13449 however I don't think this is the end of the work with this part of the code.
What comes to my mind is providing better test cases and maybe moving some of internal functions to public scope to test them (mergeElements()
I am looking at you). I would also look if during whole cutting and merging we really need so many bookmarks (at one moment I had <h1><bookmark><bookmark><bookmark></h1>
).
comment:5 Changed 10 years ago by
Also, I would like to know why mergeElements()
has to use loop to "throw" nodes from one node to another one. Shouldn't be easier just to move all childNodes of one node? I guess there might be some edge cases where this won't result in proper merge, but I can't think of any now.
comment:6 Changed 10 years ago by
Status: | assigned → review |
---|
comment:7 Changed 10 years ago by
Milestone: | → CKEditor 4.5.0 |
---|---|
Version: | → 4.5.0 Beta |
comment:8 Changed 10 years ago by
Status: | review → review_passed |
---|
comment:9 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Merged into major in git:e480e6f.
After bisecting first bad commit is: git:039322aeab