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

[GHS] List markers display incorrectly after pasting a list from Word #12361

Closed
Mgsy opened this issue Aug 30, 2022 · 6 comments · Fixed by #14145
Closed

[GHS] List markers display incorrectly after pasting a list from Word #12361

Mgsy opened this issue Aug 30, 2022 · 6 comments · Fixed by #14145
Assignees
Labels
package:html-support package:list package:paste-from-office 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

@Mgsy
Copy link
Member

Mgsy commented Aug 30, 2022

📝 Provide detailed reproduction steps (if any)

  1. Create a list in MS Word.
  2. Open a demo with GHS + allow all + Paste from Office.
  3. Paste the list from Word to CKEditor 5.

✔️ Expected result

The list markers display properly.

❌ Actual result

The list markers display incorrectly

📃 Other details

It seems to be caused by maintaining text-indent: -18pt style on <li> elements.


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

@Mgsy Mgsy added type:bug This issue reports a buggy (incorrect) behavior. package:paste-from-office squad:core Issue to be handled by the Core team. package:html-support labels Aug 30, 2022
@Mgsy Mgsy added the support:2 An issue reported by a commercially licensed client. label Apr 19, 2023
@cmak9149
Copy link

cmak9149 commented Apr 20, 2023

We run into the same issue. I have some observations that might help solving the puzzle.
Refer to the screenshot. The file is from my customer. In my test case, if I only copy from top down to "No break", the problem will not occur. If I copy beyond "Now it will break if you copy beyond this line", the bullet overlapped will occur.

On the other hand, if I copy from top down to "No break" to a new document and then add "Now it will break if you copy beyond this line" in the new file, you cannot reproduce the problem with the new document.

My conclusion is that the original document has some invisible code or setting that cause the "PasteFromWord" plugin to add "text-indent". The text indent is not part of the "li" but added in certain condition, probably related to some setting in the problematic document.

image

@Witoso
Copy link
Member

Witoso commented Apr 21, 2023

To be honest, I have a bit of a problem with this issue. And would see it more as a result of a configuration than a bug.

The HTML that we receive has the style text-indent:-18.0pt set on li element, and it's part of the clipboard's data. We don't add anything in the process:

<ol>
  <li class="MsoListParagraphCxSpFirst" style="mso-list:l0 level1 lfo1;text-indent:-18.0pt"><span lang="PL" style="mso-ansi-language:PL;mso-bidi-font-family:Calibri;mso-bidi-theme-font:minor-latin"></span><span lang="PL" style="mso-ansi-language:PL">Test<o:p></o:p></span></li>
</ol>

Paste from Office plugin cleans it but when an editor has a GHS setup that allows everything, it will allow everything. We cannot easily decide which plugin should take precedence in which case.

The best resolution, in this case, is to have a better GHS configuration that will either allow a specific set of attributes/styles/etc. or disallow some of them, in this case:

htmlSupport: {
  disallow: [
    {
      name: "li",
      styles: {"text-indent": /\-.*/}, // this will filter only negative values
    },
  ],
}

Curious what you think @Mgsy, maybe I'm not seeing something.

@Witoso Witoso added the pending:feedback This issue is blocked by necessary feedback. label Apr 24, 2023
@Mgsy
Copy link
Member Author

Mgsy commented Apr 24, 2023

Yes, definately, it is difficult to decide what should happen in this case, as General HTML Support simply preserves the markup created by MS Word, so the behavior is correct. 

However, from the UX perspective, maybe it would make sense to filter out this attribute only if the paste from office has been detected. As I can see, CKEditor 4 with allowedContent: true and Paste from Word doesn't have this problem.

OTOH, the configuration you have proposed above should be enough to handle this problem, however, I wonder if there might be some real use case for preserving negative values on <li>. If we agree that the configuration is enough, we should think about adding a new section to the documentation about troubleshooting GHS + PFO.

@Witoso
Copy link
Member

Witoso commented Apr 24, 2023

You're right, users and integrators shouldn't suffer, and they wouldn't know what attributes to disallow without some research. I like the idea it could be a part of Paste from Office.

wonder if there might be some real use case for preserving negative values on <li>.

The same case for Word. What if there are cases for which such an attribute makes sense 🤔

@Reinmar
Copy link
Member

Reinmar commented Apr 26, 2023

However, from the UX perspective, maybe it would make sense to filter out this attribute only if the paste from office has been detected. As I can see, CKEditor 4 with allowedContent: true and Paste from Word doesn't have this problem.

Probably handled here in CKE4 https://github.com/ckeditor/ckeditor4/blob/ed450ea6c632f50637da55941cc3066a27d7ea37/plugins/pastetools/filter/common.js#L263

@Witoso
Copy link
Member

Witoso commented May 4, 2023

Scope:

When pasting lists with Paste from Office, remove the text-indent style (similar to CKE4 behavior).

@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
@mmotyczynska mmotyczynska self-assigned this May 11, 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 11, 2023
arkflpc added a commit that referenced this issue May 16, 2023
…-incorrectly-when-pasting-from-word

Fix (paste-from-office): List markers should be displayed correctly after pasting a list from MS Word. Closes #12361.
@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 16, 2023
@CKEditorBot CKEditorBot added this to the iteration 64 milestone May 16, 2023
@Witoso Witoso removed the pending:feedback This issue is blocked by necessary feedback. label Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:html-support package:list package:paste-from-office 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.

6 participants