Incorrect structure created when merging block into list item on Backspace and Delete

[Boris Lykah]

Steps:

1. Create a list with two items that contain links
2. Put caret at the beginning of the second item and hit backspace

DOM result:

<a data-cke-saved-href="http://link" href="http://link">
  first
  <a data-cke-saved-href="http://link" href="http://hg">second</a>
</a>

[Boris Lykah]

The bug also happens when a link is after the list that has link in the last item.

    ..
    <li><a>link1</a></li>
  </ul>
  <a>|link2</a>

[Jakub Ś]

What you are showing us is code that you see with Chrome dev-tools. Once once you switch to Source Mode or use getData() method (used when form is submitted and data is taken from editor or when switching to source mode) - you will see that code gets fixed. Working with such links is also possible - the link plugin recognized and modified both links correctly.

All the above however can be used as workaround. The reality is that CKEditor doesn't act like native browser in this case. I have checked native code and it produced expected two links next to each other while editor nests links.

Problem can be reproduced in Webkit and Blink from CKEditor 3.6.4 rev.[7540].

[Artur Delura]

The problem was because merge list item functionality tried to look for last editable element inside previous list sibling.

Which in above example ws <a> tag. Second step was to inject data from removing list item to that element. In described case it ends up with injecting <a> element into another one which is ​not allowed.

I added conditional statement which catch this case and then move selection outside outer anchor element.

Changes in ​branch:t/12729.

[Piotr Jasiun]

You check ​here if the start container is 'a' but there could be another element inside that a (e.g. span with color), so you should check if elements path contains a instead.

Also maybe it is possible it make this fix more generic. It could check if the result html is compatible with the DTD.

[Piotrek Koszuliński]

See my comment ​https://gist.github.com/adelura/50749f9c3b32e88c7bb1#comment-1419076

The general solution here is to place the range before the end of the previous _editable_ block element. I stress the "editable" block because of cases like ​https://gist.github.com/adelura/50749f9c3b32e88c7bb1#comment-1416923 where the very first previous block contains a nested list, so position at its end is not editable.

[Artur Delura]

My first thought was to implement in similar way. But I didn't connect it with block/inline elements. Changes in ​branch:t/12729b.

[Piotrek Koszuliński]

I pushed rebased ​branch:t/12729b. It should be based on master, not major. I also pushed two small commits there.

The structure created by the patched algorithm is correct now. However, the selection is placed incorrectly between inline elements rather than staying in the inline element in which it previously was. It doesn't work on master well too, so it's not a regression fortunately.

However, the patch fixed backspace's behaviour only, while delete (forward delete) is still broken.

<ul>
	<li><a href="http://sdfsdf">sdfdsf^</a></li>
	<li><a href="http://sdfsdf">sdf</a></li>
</ul>

[Artur Delura]

I pushed changes to a ​branch:t/12729b. Problem was more general than just nesting anchors. It was caused because before mergins list items, the selection was moved into inline elements. Now it is in first block element.

I also updated tests to check selection as well. So I found out that there are issues related to placing selection after merging list items.

I tried to fix this, and current changes are in ​branch:t/12729c. I'm going to report an issue for this.

[Piotrek Koszuliński]

I rebased the branch and pushed few additional commits:

• I removed all the new code that played with bookmarks. This needs to be fixed once for all. Adding now some code which fixes few cases only means that it can be incorrect and only strengthen the incorrect behaviour. It would be confusing for someone working on this issue in the future. For instance - bookmark was created after moving to next line's editable position... why? The bookmark should be created at the caret position - not in the next line.
• I changed tests' expected patterns so they pass even though there's problem with selection which you reported in #13098.
• I fixed tests so they pass on FF and IE11 (bogus brs patterns were missing).

That would be pretty much it, but there's one red test in ​http://tests.ckeditor.dev:1030/tests/plugins/list/backspace_enterBr

[Artur Delura]

So there is a use case when end path doesn't have block element. Changes in the same branch.

[Piotrek Koszuliński]

Fixed on master with ​git:765a28c. I made the manual test totally generic, because the whole feature (bakspace/del handling around lists) needed testing.