Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paragraph shouldn't be pasted into the list item #730

Closed
Mgsy opened this issue Dec 18, 2017 · 2 comments
Closed

Paragraph shouldn't be pasted into the list item #730

Mgsy opened this issue Dec 18, 2017 · 2 comments
Assignees
Labels
type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@Mgsy
Copy link
Member

Mgsy commented Dec 18, 2017

🐞 Is this a bug report or feature request? (choose one)

  • Bug report

💻 Version of CKEditor

1.0.0-alpha2

📋 Steps to reproduce

  1. Go to http://localhost:8125/ckeditor5-core/tests/manual/article.html.
  2. Above Heading 1 create new block quote with paragraph inside.
  3. Select paragraph from the block quote and part of the Heading 1.
  4. Copy it.
  5. Select UL List item 1 and UL List item 2.
  6. Paste.

✅ Expected result

Content should be pasted properly.

❎ Actual result

<p> element has been pasted to the <li> element.

📃 Other details that might be useful

GIF
bug_cke5

Other information
OS: Windows 10, MacOS X
Browser: All browsers

This bug was reproducible before the engine refactoring.

@Mgsy Mgsy added the type:bug This issue reports a buggy (incorrect) behavior. label Dec 18, 2017
@Reinmar Reinmar added this to the iteration 14 milestone Dec 18, 2017
@Reinmar
Copy link
Member

Reinmar commented Dec 18, 2017

This is bad. Not a regression, but still important because it will lead to crashes.

@Reinmar
Copy link
Member

Reinmar commented Jan 22, 2018

This is a tricky case.

We start with:

<listItem>fo[]o</listItem>

And insert:

<blockQuote>
    <paragraph>xxx</paragraph>
</blockQuote>
<heading1>yyy</heading1>

At some point of the insertContent() method we have this:

<listItem>fo</listItem>
<blockQuote>
    <paragraph>xxx</paragraph>
</blockQuote>
<listItem>o</listItem>

So, it's a valid content. But then, we came to a point when we want to merge the block in which insertion started with the first element of the content that was inserted. Normally it works fine – if there was no <blockQuote> but only the <paragraph>xxx</paragraph> element we'd get:

<listItem>foxxx</listItem>
<heading1>yyy[]o</heading1>

However, due to the <blockQuote> we get this:

<listItem>fo<paragraph>xxx</paragraph><listItem>
<heading1>yyy[]o</listItem>

I've known about these cases for a long time but ignored them due to their complexity and because we didn't have bQs back then (so that case was not possible to reproduce).

Right now, we need to do something that will prevent incorrect merges. The easiest solution I can think if is to not merge when that might lead to an incorrect content, so a new method Schema#checkMerge() (see https://github.com/ckeditor/ckeditor5-engine/issues/1240).

The result will be then:

<listItem>fo</listItem>
<blockQuote>
    <paragraph>xxx</paragraph>
</blockQuote>
<heading1>yyy[]o</heading1>

Which I think is reasonable. There might be better solutions in terms of final result, but none seem to be feasible.

@Reinmar Reinmar assigned pomek and unassigned Reinmar Jan 22, 2018
Reinmar added a commit to ckeditor/ckeditor5-engine that referenced this issue Feb 1, 2018
Fix: `Model#insertContent()` will not merge nodes if the model after the merge would violate schema rules. Closes ckeditor/ckeditor5#730.
@Reinmar Reinmar mentioned this issue Oct 9, 2019
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

3 participants