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

New list plugin doesn't convert subsequent lists with a different type properly (aka resetting numbering, splitting lists) #12738

Closed
Tracked by #13945
mlewand opened this issue Oct 26, 2022 · 6 comments · Fixed by #14174
Assignees
Labels
package:list squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@mlewand
Copy link
Contributor

mlewand commented Oct 26, 2022

📝 Provide detailed reproduction steps (if any)

For context this is a simplified markup produced by MS Word.

  1. Copy following HTML to your clipboard:
<p style="margin-top: 0px; margin-bottom: 0px; line-height: 1">foo</p>

<p style="margin-top: 0px; margin-bottom: 0px; margin-left: 24px; line-height: 1">&nbsp;</p>
<ol style="list-style-type: decimal">
    <li>

        <p style="margin-top: 0px; margin-bottom: 10.67px; line-height: 1.05">foobar</p>
    </li>
</ol>

<p style="margin-top: 0px; margin-bottom: 10.67px; margin-left: 24px; line-height: 1.05">&nbsp;</p>
<ol style="list-style-type: lower-latin">
    <li>

        <p style="margin-top: 0px; margin-bottom: 10.67px; line-height: 1.05">foobar</p>
    </li>
    <li>

        <p style="margin-top: 0px; margin-bottom: 10.67px; line-height: 1.05">foobar</p>
    </li>
</ol>
<ol start="2" style="list-style-type: decimal">
    <li>

        <p style="margin-top: 0px; margin-bottom: 10.67px; line-height: 1.05">Decimal list item again</p>
    </li>
    <li>

        <p style="margin-top: 0px; margin-bottom: 10.67px; line-height: 1.05">Decimal list item again
        </p>
    </li>
</ol>

<p style="margin-top: 0px; margin-bottom: 10.67px; margin-left: 48px; line-height: 1.05">&nbsp;</p>
<ol style="list-style-type: lower-latin">
    <li>

        <p style="margin-top: 0px; margin-bottom: 10.67px; line-height: 1.05">foobar</p>
    </li>
    <li>

        <p style="margin-top: 0px; margin-bottom: 10.67px; line-height: 1.05">foobar</p>
    </li>
</ol>
<ul style="list-style-type: disc">
    <li>

        <p style="margin-top: 0px; margin-bottom: 10.67px; line-height: 1.05">foobar</p>
    </li>
</ul>
<ol start="3" style="list-style-type: lower-latin">
    <li>

        <p style="margin-top: 0px; margin-bottom: 10.67px; line-height: 1.05">foobar
        </p>
    </li>
</ol>
  1. Open a demo that has new list (a.k.a. document list) implementation in it. Import from Word guide is a good example.
  2. Open js dev console.
  3. Execute editor.setData( `<copiedMarkupHere>` ).

✔️ Expected result

List with two "Decimal list item again" items should be a numbered list.

❌ Actual result

List goes on as alphanum list.


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@mlewand mlewand added type:bug This issue reports a buggy (incorrect) behavior. package:list squad:core Issue to be handled by the Core team. labels Oct 26, 2022
@Reinmar
Copy link
Member

Reinmar commented Oct 26, 2022

There's a chance we're running here into a limitation of the current implementation – that it does not allow having two subsequent lists of the same type.

@niegowski or @arkflpc, could you confirm that I'm right? Or perhaps this is indeed a bug (as opposed to a missing feature)?

@niegowski
Copy link
Contributor

There's a chance we're running here into a limitation of the current implementation – that it does not allow having two subsequent lists of the same type.

You are right. We even implemented a post-fixer to make sure this would not break in RTC:

// Make sure that all items in a single list (items at the same level & listType) have the same properties.
documentListEditing.on( 'postFixer', ( evt, { listNodes, writer } ) => {

I could imagine a case where one user is modifying a style of the list and some other user is at the same time adding a new list item. 

This probably could be someday resolved by adding listId attribute to mark strict boundaries for lists, but currently, it's a missing feature.

@jacekbogdanski
Copy link
Member

jacekbogdanski commented Oct 27, 2022

Just theoretically, how feasible would be that fix if IFW would mark lists with an additional attribute? Like a unique ID or something, so it's possible to distinguish different lists? Would that help?

@niegowski
Copy link
Contributor

Just theoretically, how feasible would be that fix if IFW would mark lists with an additional attribute? Like a unique ID or something, so it's possible to distinguish different lists? Would that help?

It's not the input problem, it's internally stored in the model (in an input-compatible way) and fixed by the post-fixer.

@martynawierzbicka martynawierzbicka added the support:2 An issue reported by a commercially licensed client. label Dec 12, 2022
@Reinmar Reinmar changed the title New list plugin doesn't convert subsequent lists with a different type properly (a.k.a. document lists) New list plugin doesn't convert subsequent lists with a different type properly (aka resetting numbering, splitting lists) Apr 12, 2023
@Reinmar
Copy link
Member

Reinmar commented Apr 12, 2023

Very old DUP of this ticket: #3000.

@CKEditorBot CKEditorBot added status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Apr 26, 2023
@Witoso
Copy link
Member

Witoso commented May 4, 2023

Scope

  1. Only for DocumentList plugin.
  2. Requires a new docs page / section.

Enablement

New plugin that is added by the user.

Behaviour

  1. Always show two or more separate lists when we have two or more separate lists in the data.
  2. List separator - create an element in the model (not visible, margins will be probably larger) - to explore.
  3. Check the behavior for backspace and forward delete (problems may appear).

Two ul’s next to each other:

<ul>
	<li>LOREM</li>
	<li>IPSUM</li>
</ul>
<ul>
	<li>LOREM</li>
	<li>IPSUM</li>
</ul>

Outcome: This should not be merged.


Two ul’s next to each other, different styles (only list-style-type: or more styles?):

<ul style="list-style-type: square;">
	<li>LOREM</li>
	<li>IPSUM</li>
</ul>
<ul style="list-style-type: disc;">
	<li>LOREM</li>
	<li>IPSUM</li>
</ul>

Outcome: This should not be merged.

ol's

Two ol’s next to each other:

<ol>
	<li>LOREM</li>
	<li>IPSUM</li>
</ol>
<ol>
	<li>LOREM</li>
	<li>IPSUM</li>
</ol>

Outcome: This should not be merged.


Two ol’s next to each other, different styles (only list-style-type: or more styles?).
Probably not relevant as it will be handled by the case above anyway.

<ol>
	<li>LOREM</li>
	<li>IPSUM</li>
</ol>
<ol style="list-style-type:decimal-leading-zero;">
	<li>LOREM</li>
	<li>IPSUM</li>
</ol>

Outcome: This should not be merged.

Real life examples

@Witoso Witoso closed this as completed May 4, 2023
@Witoso Witoso reopened this May 4, 2023
@CKEditorBot CKEditorBot added the status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. label May 8, 2023
@filipsobol filipsobol self-assigned this May 10, 2023
@CKEditorBot CKEditorBot added status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels May 10, 2023
niegowski added a commit that referenced this issue May 23, 2023
…rator-plugin

Feature (list): Add the `AdjacentListsSupport` plugin to separate lists of the same type. Closes #12738.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label May 23, 2023
@CKEditorBot CKEditorBot added this to the iteration 64 milestone May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:list squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants