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

Wrong content inside editor (<div> with TrackChanges) #7720

Open
kaluginserg opened this issue Jul 27, 2020 · 11 comments
Open

Wrong content inside editor (<div> with TrackChanges) #7720

kaluginserg opened this issue Jul 27, 2020 · 11 comments
Labels
pending:feedback This issue is blocked by necessary feedback. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@kaluginserg
Copy link

kaluginserg commented Jul 27, 2020

📝 Provide detailed reproduction steps (if any)

  1. usual build with some plugins:
    class Editor extends ClassicEditor {}
    Editor.builtinPlugins = [
        Bold,
        Essentials,
        Heading,
        List,
        Paragraph,
        RealTimeCollaborativeEditing,
        RealTimeCollaborativeTrackChanges,
    ];
    export Editor;
  2. initiate as usual:
    Editor.create(document.querySelector('#editor'), {
            initialData: "",
            plugins: [
                'Essentials','Paragraph','Bold','List','Heading','RealTimeCollaborativeEditing','RealTimeCollaborativeTrackChanges',
            ],
            toolbar: [
                'heading','bold','bulletedlist','trackChanges'
            ],
            cloudServices: {
                tokenUrl: () => Promise.resolve(getJwtCkEditorToken()),
                webSocketUrl: CKE_WEBSOCKET_URL,
            },
            collaboration: {channelId: "test-text-channel-123"},
        })
        .then(editor => {
            window.editor = editor;
        });

A) option:

editor.data.set(`<suggestion id="e05fe0ef7a56be8063bb94c6caf6e1b3d:USER1" suggestion-type="formatBlock:fz6284npvaph" type="start"></suggestion><p>asd<suggestion id="e05fe0ef7a56be8063bb94c6caf6e1b3d:USER1" suggestion-type="formatBlock:fz6284npvaph" type="end"></suggestion></p>`,
{suppressErrorInCollaboration: true});
editor.getData();

B) option (just div extra wrapper for initialData)

editor.data.set(`<div><suggestion id="e05fe0ef7a56be8063bb94c6caf6e1b3d:USER1" suggestion-type="formatBlock:fz6284npvaph" type="start"></suggestion><p>asd<suggestion id="e05fe0ef7a56be8063bb94c6caf6e1b3d:USER1" suggestion-type="formatBlock:fz6284npvaph" type="end"></suggestion></p></div>`,
{suppressErrorInCollaboration: true});
editor.getData();

✔️ Expected result

The both results(A and B) should be the same or B should throw exception.

❌ Actual result

A =>

<suggestion id="e05fe0ef7a56be8063bb94c6caf6e1b3d:USER1" suggestion-type="formatBlock:fz6284npvaph" type="start"></suggestion><p>asd<suggestion id="e05fe0ef7a56be8063bb94c6caf6e1b3d:USER1" suggestion-type="formatBlock:fz6284npvaph" type="end"></suggestion></p>

B =>

<p>&nbsp;</p><p>asd</p>
  • Unexpected new paragraph! ❌
  • Unexpected removed suggestion! ❌

Result should be the same as A or an exception should be thrown.

📃 Other details

  • Browser: Chrome
  • OS: MacOs
  • CKEditor version: 19.1
  • CKEditor Cloud Services REST API v3

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

@kaluginserg kaluginserg added the type:bug This issue reports a buggy (incorrect) behavior. label Jul 27, 2020
@Reinmar
Copy link
Member

Reinmar commented Jul 28, 2020

cc @scofalik

@scofalik
Copy link
Contributor

scofalik commented Jul 30, 2020

I confirm that in option B additional paragraph is created. I'd guess this is because the <div> is not empty, and autoparagraphing changes it to <p>, or similar mechanism is fired. I didn't debug it because it won't be a problem since the next release which will be available in a few days.

In the next release we are introducing a new way to convert markers. Your new data should look like this:

<div><p data-suggestion-start-before="insertion:suggestion-1:user-2">asd<suggestion-end name="insertion:suggestion-1:user-2"></suggestion-end></p></div>

Now, markers that are right before/after an element are stored as attributes. Only markers "in text" are represented by tags. I hope this will resolve your problem.

I've tested loading the above HTML and it worked as expected.

@lslowikowska lslowikowska added the support:2 An issue reported by a commercially licensed client. label Jul 30, 2020
@kaluginserg
Copy link
Author

@scofalik, @lslowikowska - Wow.
It is very very big changes for us, I mean - changing suggestions markup approach...
As I see in your markup example - there will be no more <suggestion></suggestion> tags???
In our application we use them for generate word documents, and generate new suggestions on server side (for new documents), and etc... ((((
Am I right understand that all our existing documents in db with old suggestions must be updated?

@scofalik
Copy link
Contributor

scofalik commented Jul 30, 2020

I understand that this is a big change, however it was a must. We are certainly very sorry to introduce such breaking changes. Here is the issue that is related to that change: #7556. The last post in the issue discusses "migration" a bit.

Other notes in this regard:

  1. Yes, this is a breaking change if you process editor data. Unfortunately, the new editor data will be harder to process.
  2. AFAIK, there are no critical issues (like security or major editor problems) fixed in this iteration, so you can wait several days before updating the editor.
  3. The change is backward compatible in a way that you will be able to load your old documents. The editor will load all the data correctly. However, if you save the editor data, you will get it in the new format. So you don't need to process all the documents in your database to fit them into the new format.

@scofalik
Copy link
Contributor

I think I could also write instruction for those who process editor data on how their tools can be changed to fit the new data structure.

@scofalik
Copy link
Contributor

@kaluginserg We are also working on the solution that will let you get the data with suggestions accepted or rejected straight from the editor. This should be ready in the next iteration (at the end of August). Maybe this feature would also solve some of your problems?

@kaluginserg
Copy link
Author

Maybe. We hope)
Currently - trackChanges API is poorly suited for our integrating purposes. And too coupled to the client side. Because of that, currently we have to do lot of extra-work (facades and adapters) for handle track-changes and comments on server side and on client side.

But I am not sure that I right understand what you mean about "get the data with accepted/rejected suggestions"..

@scofalik
Copy link
Contributor

This is an off-topic discussion for this issue so let's not follow it further. The feature will simply let you get:

<p>Foo bar baz</p>

Instead of:

<p>Foo <suggestion-start...></suggestion-start>bar<suggestion-end></suggestion-end> baz</p>

Of course, it will apply to all types of suggestion.

Same for discarding suggestions, you will be able to get:

<p>Foo baz</p>

straight from the editor.

@Reinmar Reinmar added the pending:feedback This issue is blocked by necessary feedback. label Aug 3, 2020
@kaluginserg
Copy link
Author

This is an off-topic discussion for this issue so let's not follow it further. The feature will simply let you get:

These changes - won't help us. Even I am not sure that we will use this feature. Because we need store all metadata in the markup (we do many server side manipulations with accepting/rejecting - we have parsers for all cke5 generated markup...).

I think I could also write instruction for those who process editor data on how their tools can be changed to fit the new data structure.

@scofalik, @lslowikowska - is there a date, when we can look at these instructions?

@scofalik
Copy link
Contributor

scofalik commented Aug 3, 2020

I've just finished writing: #7556 (comment)

I hope this will be helpful.

@scofalik
Copy link
Contributor

scofalik commented Aug 3, 2020

Edit: I edited splitMarkerName function (now parseSuggestionName) to make it less confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending:feedback This issue is blocked by necessary feedback. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

4 participants