Bug 49288 – REGRESSION (Safari 5?): Pasting a line into textarea inserts two newlines

• Component: HTML Editing
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

Steps to reproduce:
1. Select and copy this whole line in Bugzilla (e.g. via triple-click and Cmd+C).
2. Paste it into additional comments field several times.

There are empty lines between pasted lines, suggesting that two newlines are getting pasted instead of one.

This is rather annoying, I hit this multiple times a day in Bugzilla. This only started to happen time ago - assuming that was Safari 5, but I'm not sure.

[Marijn Haverbeke]

CodeMirror (http://codemirror.net) uses a textarea in a way that makes this bug extra visible. I've had several people report it as a bug in CodeMirror in the past few months [1][2][3], and in fact ran into it writing this very comment in the bug tracker.

This seems a serious, obviously reproduceable bug. I hope it's still on someone's list? (Has been sitting idle in the bug tracker for a long time now.)

[1]: https://github.com/marijnh/CodeMirror2/issues/598
[2]: https://github.com/marijnh/CodeMirror2/issues/474
[3]: http://groups.google.com/group/codemirror/browse_thread/thread/9b42dec8a17a2dc1

[Simon Fraser (smfr)]

This is also important for Adobe's Brackets editor.

[Radar WebKit Bug Importer]

<rdar://problem/11746339>

[Ryosuke Niwa]

Fix:

Index: Source/WebCore/editing/ReplaceSelectionCommand.cpp
===================================================================
--- Source/WebCore/editing/ReplaceSelectionCommand.cpp	(revision 121106)
+++ Source/WebCore/editing/ReplaceSelectionCommand.cpp	(working copy)
@@ -123,9 +123,12 @@
     // The two positions above are the same visual position, but we want to stay in the same block.
     Node* enclosingBlockNode = enclosingBlock(pos.containerNode());
     for (Position nextPosition = pos; nextPosition.containerNode() != enclosingBlockNode; pos = nextPosition) {
+        if (lineBreakExistsAtPosition(pos))
+            break;
+
         if (pos.containerNode()->nonShadowBoundaryParentNode())
             nextPosition = positionInParentAfterNode(pos.containerNode());
-        
+
         if (nextPosition == pos 
             || enclosingBlock(nextPosition.containerNode()) != enclosingBlockNode
             || VisiblePosition(pos) != VisiblePosition(nextPosition))

I'll post a patch tomorrow.

[Ryosuke Niwa]

Maybe caused by http://trac.webkit.org/changeset/55762 ?

[Tony Chang]

(In reply to comment #5)
> Maybe caused by http://trac.webkit.org/changeset/55762 ?

Seems possible. I probably didn't consider the impact on plain text by moving to an equivalent visual position.

[Ryosuke Niwa]

Created attachment 149714 [details]
Fixes the bug

[Marijn Haverbeke]

Thank you for figuring this out!

[Tony Chang]

Comment on attachment 149714 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=149714&action=review

> LayoutTests/ChangeLog:8
> +        Add a test regerssion test for copying & pasting a line in pre into textarea twice.

Typo regression and I think you don't mean to say test twice.

[Ryosuke Niwa]

Committed r121357: <http://trac.webkit.org/changeset/121357>