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

Pasting paragraphs sometimes adds unexpected linebreaks #4602

Closed
mejo- opened this issue Jul 28, 2023 · 6 comments · Fixed by #4872
Closed

Pasting paragraphs sometimes adds unexpected linebreaks #4602

mejo- opened this issue Jul 28, 2023 · 6 comments · Fixed by #4872
Assignees
Labels
0. Needs triage bug Something isn't working feature: formatting Features related to text formatting and node types

Comments

@mejo-
Copy link
Member

mejo- commented Jul 28, 2023

Describe the bug
When pasting paragraphs to a document, sometimes unexpected linebreaks are added in between the paragraph. As soon as you do one change in the paragraph, all unexpected linebreaks get deleted at once.

As an example, paste the first 3-4 paragraphs of the wikipedia article about Fairy Tales into a document.

To Reproduce
Steps to reproduce the behavior:

  1. Open a document, paste the example paragraph from above.
  2. See unexpected linebreaks
  3. Add one character anywhere in the paragraph
  4. The the unexpected linebreaks disappearing.
@mejo- mejo- added bug Something isn't working feature: formatting Features related to text formatting and node types 0. Needs triage bug: regression labels Jul 28, 2023
@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Office team Jul 28, 2023
@juliusknorr
Copy link
Member

This seems to come from the white-space: pre-wrap; that is added to the paragraph. Might be coming from @tiptap/vue-2/src/NodeViewContent.ts

@mejo-
Copy link
Member Author

mejo- commented Aug 2, 2023

Indeed, that's the reason for this behaviour. I wonder whether we should use our own implementation of NodeViewContent without white-space: pre-wrap? @juliushaertl and @max-nextcloud do you see any reason why we would need white-space: pre-wrap in NodeViewContent?

@juliusknorr
Copy link
Member

This seems to come from the prosemirror stylesheet recommentations ad would also warn in ProseMirror/prosemirror-view@b8b814c, but i think it the original paragraph node doesn't have it we can also drop it here.

ProseMirror/prosemirror#981 might give a bit of reasoning for what it is needed

@juliusknorr
Copy link
Member

Worth to double check against this logic

key: new PluginKey('pasteEventHandler'),
props: {
handleDOMEvents: {
mouseup(_, event) {
shiftKey = event.shiftKey
return false
},
},
handleKeyDown(_, event) {
shiftKey = event.shiftKey
return false
},
clipboardTextParser(str, $context, _, view) {
const parser = DOMParser.fromSchema(view.state.schema)
const doc = document.cloneNode(false)
const dom = doc.createElement('div')
if (shiftKey) {
// Treat single newlines as linebreaks and double newlines as paragraph breaks when pasting as plaintext
dom.innerHTML = '<p>' + str.replaceAll('\n', '<br />').replaceAll('<br /><br />', '</p><p>') + '</p>'
} else {
dom.innerHTML = markdownit.render(str)
}
return parser.parseSlice(dom, { preserveWhitespace: true, context: $context })
},
},

@mejo- mejo- moved this from 🏗️ In progress to 📄 To do (~10 entries) in 📝 Office team Oct 12, 2023
@max-nextcloud
Copy link
Collaborator

Experiments

replace the preserveWhitespace: full in parseHtml with true

This will fix the unexpected line breaks but at the same time also breaks the tests for preserving line breaks in markdown.

in addition to #4624 I also remove whiteSpace setting in NodeViewWrapper

Removing this broke the #4624 fix as now whiteSpace was inheriting pre-wrap again from the .Prosemirror tag.

Conclusion

We actually have some conflicting interest here:

  • preserve whitespace (in md files, when pasting lists of plaintext separate by empty lines)
  • get rid of unexpected linebreaks when pasting html content.

However it's hard to destinguish pasting html from the initial load as we are also inserting html there.

@max-nextcloud
Copy link
Collaborator

Looks like prosemirror has a transformPastedHtml option that we could use to get rid of newlines outside of pre tags:

https://github.com/ProseMirror/prosemirror-view/blob/a93d102a121585d2244c7d23ba8383e277cf4aac/src/clipboard.ts#L64

max-nextcloud added a commit that referenced this issue Oct 17, 2023
By default html does not display whitespace but text does.
Remove newlines when pasting to ensure consistency with the previous display.

Keep the newlines when they are also displayed in the source.

* Resolves #4602.

Signed-off-by: Max <[email protected]>
max-nextcloud added a commit that referenced this issue Oct 17, 2023
By default html does not display whitespace but text does.
Remove newlines when pasting to ensure consistency with the previous display.

Keep the newlines when they are also displayed in the source.

* Resolves #4602.

Signed-off-by: Max <[email protected]>
@max-nextcloud max-nextcloud self-assigned this Oct 17, 2023
@max-nextcloud max-nextcloud moved this from 📄 To do (~10 entries) to 🏗️ In progress in 📝 Office team Oct 17, 2023
max-nextcloud added a commit that referenced this issue Oct 18, 2023
By default html does not display whitespace but text does.
Remove newlines when pasting to ensure consistency with the previous display.

Keep the newlines when they are also displayed in the source.

* Resolves #4602.

Signed-off-by: Max <[email protected]>
mejo- pushed a commit that referenced this issue Oct 24, 2023
By default html does not display whitespace but text does.
Remove newlines when pasting to ensure consistency with the previous display.

Keep the newlines when they are also displayed in the source.

* Resolves #4602.

Signed-off-by: Max <[email protected]>
Signed-off-by: Jonas <[email protected]>
@github-project-automation github-project-automation bot moved this from 🏗️ In progress to ☑️ Done in 📝 Office team Oct 24, 2023
backportbot-nextcloud bot pushed a commit that referenced this issue Oct 24, 2023
By default html does not display whitespace but text does.
Remove newlines when pasting to ensure consistency with the previous display.

Keep the newlines when they are also displayed in the source.

* Resolves #4602.

Signed-off-by: Max <[email protected]>
Signed-off-by: Jonas <[email protected]>
backportbot-nextcloud bot pushed a commit that referenced this issue Oct 24, 2023
By default html does not display whitespace but text does.
Remove newlines when pasting to ensure consistency with the previous display.

Keep the newlines when they are also displayed in the source.

* Resolves #4602.

Signed-off-by: Max <[email protected]>
Signed-off-by: Jonas <[email protected]>
mejo- pushed a commit that referenced this issue Oct 26, 2023
By default html does not display whitespace but text does.
Remove newlines when pasting to ensure consistency with the previous display.

Keep the newlines when they are also displayed in the source.

* Resolves #4602.

Signed-off-by: Max <[email protected]>
Signed-off-by: Jonas <[email protected]>
mejo- pushed a commit that referenced this issue Oct 26, 2023
By default html does not display whitespace but text does.
Remove newlines when pasting to ensure consistency with the previous display.

Keep the newlines when they are also displayed in the source.

* Resolves #4602.

Signed-off-by: Max <[email protected]>
Signed-off-by: Jonas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage bug Something isn't working feature: formatting Features related to text formatting and node types
Projects
Archived in project
3 participants