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.

Sign up for GitHub

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

Jump to bottom

Extra paragraph and break elements when copy/pasting from Google Docs #10217

Closed
AleksBeliun opened this issue Jul 26, 2021 · 7 comments · Fixed by #12033 or #12366
Closed

Extra paragraph and break elements when copy/pasting from Google Docs #10217

AleksBeliun opened this issue Jul 26, 2021 · 7 comments · Fixed by #12033 or #12366
Assignees
@niegowski
Labels
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. type:regression This issue reports a bug that was not present in the previous releases.
Milestone
iteration 57 (v35...

Comments

@AleksBeliun
Copy link

AleksBeliun commented Jul 26, 2021 •
edited
Loading

📝 Provide detailed reproduction steps

  1. Go to https://docs.google.com/document/d/1abo1Ca_RHEmuXnAX6k_z0itHihQb3g9rJRM3x4vQ25w/edit
  2. Copy some text
  3. Go to https://ckeditor.com/docs/ckeditor5/latest/features/basic-styles.html#demo
  4. Paste the text

✔️ Expected result

image

image

❌ Actual result

image

image

📃 Other details

  • Browser: Tested on Google Chrome Version 91.0.4472.164, Firefox 91.0b7 (64-bit), Safari Version 14.1.1 (16611.2.7.1.4)
  • OS: macOS Big Sur Version 11.4
  • First affected CKEditor version: 26.0.0
  • Installed CKEditor plugins:
    "@ckeditor/ckeditor5-alignment": "^29.0.0",
    "@ckeditor/ckeditor5-autoformat": "^29.0.0",
    "@ckeditor/ckeditor5-basic-styles": "^29.0.0",
    "@ckeditor/ckeditor5-block-quote": "^29.0.0",
    "@ckeditor/ckeditor5-build-classic": "^29.0.0",
    "@ckeditor/ckeditor5-core": "^29.0.0",
    "@ckeditor/ckeditor5-dev-utils": "^25.2.6",
    "@ckeditor/ckeditor5-dev-webpack-plugin": "^25.2.6",
    "@ckeditor/ckeditor5-editor-classic": "^29.0.0",
    "@ckeditor/ckeditor5-essentials": "^29.0.0",
    "@ckeditor/ckeditor5-font": "^29.0.0",
    "@ckeditor/ckeditor5-heading": "^29.0.0",
    "@ckeditor/ckeditor5-highlight": "^29.0.0",
    "@ckeditor/ckeditor5-indent": "^29.0.0",
    "@ckeditor/ckeditor5-link": "^29.0.0",
    "@ckeditor/ckeditor5-list": "^29.0.0",
    "@ckeditor/ckeditor5-media-embed": "^29.0.0",
    "@ckeditor/ckeditor5-paragraph": "^29.0.0",
    "@ckeditor/ckeditor5-paste-from-office": "^29.0.0",
    "@ckeditor/ckeditor5-react": "^3.0.2",
    "@ckeditor/ckeditor5-remove-format": "^29.0.0",
    "@ckeditor/ckeditor5-special-characters": "^29.0.0",
    "@ckeditor/ckeditor5-table": "^29.0.0",
    "@ckeditor/ckeditor5-theme-lark": "^29.0.0",
    "@ckeditor/ckeditor5-typing": "^29.0.0",
    "@ckeditor/ckeditor5-ui": "^29.0.0",

Built from source for React with Webpack as per official guideline.

Trying to update from 23.1.0 to the latest. Currently updated up to 25.0.0

If you'd like to see this fixed sooner, add a 👍 reaction to this post.
@AleksBeliun AleksBeliun added the type:bug This issue reports a buggy (incorrect) behavior. label Jul 26, 2021
@FilipTokarski FilipTokarski added package:paste-from-office squad:core Issue to be handled by the Core team. labels Jul 28, 2021
@FilipTokarski
Copy link
Member

Hi, thanks for the report. It's reproducible on paste from GDocs docs since version 26.0.0, so seems like a regression we missed.

@FilipTokarski FilipTokarski added the type:regression This issue reports a bug that was not present in the previous releases. label Jul 28, 2021
@FilipTokarski FilipTokarski mentioned this issue Jul 28, 2021
Closed
@Reinmar
Copy link
Member

Reinmar commented Aug 25, 2021

Thanks for checking it @FilipTokarski. Could you try to bisect it? It may help prioritizing it.

@martynawierzbicka martynawierzbicka added the support:2 An issue reported by a commercially licensed client. label Jan 24, 2022
@oleq
Copy link
Member

oleq commented Jan 26, 2022

Input:

Result in v31.1.0:

@Reinmar
Copy link
Member

Reinmar commented Jan 26, 2022

In the above case, it'd make sense for this paragraph to be empty (have no <softBreak /> in the model).

@arkflpc
Copy link
Contributor

arkflpc commented Jan 26, 2022

Guilty commit: 74d1d15

@niegowski niegowski self-assigned this Jul 6, 2022
@CKEditorBot CKEditorBot added the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Jul 6, 2022
@niegowski niegowski mentioned this issue Jul 7, 2022
Merged
Reinmar added a commit that referenced this issue Jul 20, 2022
@Reinmar
Merge pull request #12033 from ckeditor/ck/10217
394a19b 
Fix (enter): A `<br>` element between blocks should be converted to an empty paragraph. Along with the fix we revised the entire implementation and improved it a number of cases. Closes #10217.

Other (engine): A mechanism responsible for removing bogus `<br>`s was moved from `DomConverter` to a plugin space (namely: `ShiftEnter`) and is now run on a conversion layer. See #10217.

Test (clipboard): Added test for pasting content with `<br>` element between blocks. See #10217.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Jul 20, 2022
@Reinmar Reinmar added this to the iteration 55 milestone Jul 20, 2022
@Reinmar
Copy link
Member

Reinmar commented Jul 29, 2022

Unfortunately, we had to revert this change. See #12183.

@Reinmar Reinmar reopened this Jul 29, 2022
@Reinmar Reinmar removed this from the iteration 55 milestone Jul 29, 2022
Reinmar added a commit that referenced this issue Jul 29, 2022
@Reinmar
Merge pull request #12183 from ckeditor/revert-12033-ck/10217
8c0c3a5 
Revert: Reverted a fix for #10217.
@arkflpc arkflpc closed this as completed Aug 29, 2022
@arkflpc arkflpc reopened this Aug 29, 2022
@Reinmar
Copy link
Member

Reinmar commented Aug 29, 2022

Let's try to narrow the scope and target only this markup coming specifically from Google Docs.

The fix may land in the PFO package and use the normalization step where we clean up those specific weird HTML thingies that Google Docs or Word produce.

This way we'll avoid touching the more fragile logic of <br> conversion.

@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 Aug 29, 2022
@niegowski niegowski self-assigned this Aug 30, 2022
@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 Aug 30, 2022
@niegowski niegowski mentioned this issue Aug 30, 2022
Merged
arkflpc added a commit that referenced this issue Sep 1, 2022
@arkflpc
Merge pull request #12366 from ckeditor/ck/10217-paste-from-office
f2efcee 
Fix (paste-from-office): Extra line breaks should not be generated while pasting from Google Docs. Closes #10217.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Sep 1, 2022
@CKEditorBot CKEditorBot added this to the iteration 57 milestone Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@niegowski niegowski

Labels
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. type:regression This issue reports a bug that was not present in the previous releases.
Projects
None yet
Milestone
iteration 57 (v35.2.0)
8 participants
@Reinmar @oleq @niegowski @arkflpc @CKEditorBot @FilipTokarski @AleksBeliun @martynawierzbicka