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

HTML comments: comments located between some HTML tags are repositioned or lost #10118

Open
psmyrek opened this issue Jul 9, 2021 · 6 comments
Labels
domain:v4-compatibility This issue reports a CKEditor 4 feature/option that's missing in CKEditor 5. package:html-support squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@psmyrek
Copy link
Contributor

psmyrek commented Jul 9, 2021

Provide a description of the task

Steps to reproduce:

  1. Open HtmlComment manual test: http://localhost:8125/ckeditor5-html-support/tests/manual/htmlcomment.html.
  2. Set the document content to:
<figure class="table">
    <!-- C1 -->
    <table>
        <!-- C2 -->
        <tbody>
            <!-- C3 -->
            <tr>
                <!-- C4 -->
                <td>
                    <!-- C5 -->Table cell<!-- C6 -->
                </td>
                <!-- C7 -->
            </tr>
            <!-- C8 -->
        </tbody>
        <!-- C9 -->
    </table>
    <!-- C10 -->
</figure>
  1. Call editor.getData(). A few comments, originally located between <figure> and <table>, <table> and <tbody> or <tbody> and <tr> , are moved after the closing </table> tag.
<figure class="table">
    <table>
        <tbody>
            <tr>
                <!-- C4 -->
                <td>
                    <!-- C5 -->Table cell<!-- C6 -->
                </td>
                <!-- C7 -->
            </tr>
        </tbody>
    </table>
    <!-- C10 --><!-- C1 --><!-- C9 --><!-- C8 --><!-- C3 --><!-- C2 -->
</figure>

It is also easily reproducible using SourceEditing:

comments.repositioned.mp4
@psmyrek psmyrek added type:task This issue reports a chore (non-production change) and other types of "todos". squad:compat domain:v4-compatibility This issue reports a CKEditor 4 feature/option that's missing in CKEditor 5. labels Jul 9, 2021
@psmyrek psmyrek changed the title HTML comments: comments located between some HTML tags are repositioned HTML comments: comments located between some HTML tags are repositioned or lost Jul 12, 2021
@psmyrek
Copy link
Contributor Author

psmyrek commented Jul 12, 2021

Similar issue is with the List plugin, but in this case some comments are totally lost.

Example: after initializing the editor with the following content:

<!-- c1 -->
<ol>
    <!-- c2 -->
    <li>
        <!-- c3 -->
        Ordered list item
        <!-- c4 -->
    </li>
    <!-- c5 -->
</ol>
<!-- c6 -->
<ul>
    <!-- c7 -->
    <li>
        <!-- c8 -->
        Unordered list item
        <!-- c9 -->
    </li>
    <!-- c10 -->
</ul>
<!-- c11 -->

the editor.getData() returns:

<!-- c1 -->
<ol>
    <li>
        <!-- c3 -->
        Ordered list item
        <!-- c4 -->
    </li>
</ol>
<!-- c6 -->
<ul>
    <li>
        <!-- c8 -->
        Unordered list item
        <!-- c9 -->
    </li>
</ul>
<!-- c11 -->

Comments <!-- c2 -->, <!-- c5 -->, <!-- c7 --> and <!-- c10 --> are lost.

@Reinmar Reinmar added this to the nice-to-have milestone Jul 13, 2021
@Reinmar Reinmar added squad:core Issue to be handled by the Core team. and removed squad:compat labels Sep 27, 2021
@Mgsy Mgsy added the support:2 An issue reported by a commercially licensed client. label Oct 6, 2021
@pomek pomek removed this from the nice-to-have milestone Feb 21, 2022
@star-szr
Copy link

star-szr commented Nov 15, 2023

I am currently trying to solve this issue with lists, specifically the newer DocumentList plugin. I'm working on a project where it's common for <li> items to be commented out. Would anyone be able to share some pointers on how to preserve these, whether in the HtmlComment plugin/domconverter (happy to contribute if I can help figure it out) OR with a custom plugin?

So far I've only been able to figure out that these comments (like the c2, c5, c7, c10 from the previous comment) don't make it to the upcast converter in HtmlComment, and I'm not yet sure why.

Example markup:

<ul>
    <li>
        one
    </li>
<!--
    <li>
        two
    </li>
-->
</ul>

This turns into:

<ul>
    <li>
        one
    </li>
</ul>

@Witoso
Copy link
Member

Witoso commented Nov 16, 2023

@niegowski any ideas here?

@star-szr
Copy link

star-szr commented Nov 30, 2023

I have a potential solution for lists, I can propose a fix in a pull request. I started looking into tables to see if it could be solved in a similar way but didn't get very far yet.

Update: In some cases the comments will still get repositioned (when at the start/end of the list from what I can see), but they would not be lost. I haven't looked into any solution for repositioning yet since my main concern is data loss. I will put up a basic PR later referencing this issue, and I can work on some tests next week.

Update again: Actually List and DocumentList both need updates, I have been focused on DocumentList but will see if List can be updated as well.

@imdgtm
Copy link

imdgtm commented Feb 2, 2024

@star-szr Any update on comment repositioned within table?

@star-szr
Copy link

star-szr commented Feb 2, 2024

@star-szr Any update on comment repositioned within table?

Hi, it's not needed for the project that I am working on so I am not planning to work on that.

star-szr added a commit to star-szr/ckeditor5 that referenced this issue Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:v4-compatibility This issue reports a CKEditor 4 feature/option that's missing in CKEditor 5. package:html-support squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants