RESOLVED FIXED 79305
REGRESSION(r99076): WebKit pastes the trailing newline into a single-line text field
https://bugs.webkit.org/show_bug.cgi?id=79305
Summary REGRESSION(r99076): WebKit pastes the trailing newline into a single-line tex...
Ryosuke Niwa
Reported 2012-02-22 16:53:11 PST
Created attachment 128334 [details] demo Reproduction steps: 1. Open the attachment 2. Select all in textarea 3. Copy 4. Paste in input Expected result: See "abc def " in the text field Actual result: Get "abc def\n" in the text field
Attachments
demo (336 bytes, text/html)
2012-02-22 16:53 PST, Ryosuke Niwa
no flags
Fixes the bug (6.01 KB, patch)
2012-02-22 17:37 PST, Ryosuke Niwa
tony: review+
Ryosuke Niwa
Comment 2 2012-02-22 17:37:10 PST
Created attachment 128348 [details] Fixes the bug
Tony Chang
Comment 3 2012-02-23 09:58:08 PST
Comment on attachment 128348 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=128348&action=review > Source/WebCore/editing/ReplaceSelectionCommand.cpp:188 > + // New lines may have been pruned by text field's before text inserted event. > + m_hasInterchangeNewlineAtStart = false; > + m_hasInterchangeNewlineAtEnd = false; It looks like these can be set to true by removeInterchangeNodes(). Do we need to reset these values after the call to removeInterchangeNodes() 2 lines down? I guess it's not obvious why we set the values at this spot. > LayoutTests/resources/dump-as-markup.js:242 > +Markup._getShadowHostIfPossible = function (node, depth) Nit: Remove the space between function and open parenthesis.
Ryosuke Niwa
Comment 4 2012-02-23 10:16:51 PST
Comment on attachment 128348 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=128348&action=review Thanks for the review. >> Source/WebCore/editing/ReplaceSelectionCommand.cpp:188 >> + m_hasInterchangeNewlineAtEnd = false; > > It looks like these can be set to true by removeInterchangeNodes(). Do we need to reset these values after the call to removeInterchangeNodes() 2 lines down? I guess it's not obvious why we set the values at this spot. No. The point is that removeInterchangeNodes sets these variables to true but never resets to false when they should. Okay, on my second thought, these resets should happen inside removeInterchangeNodes. Will move the assignments before lading the patch. >> LayoutTests/resources/dump-as-markup.js:242 >> +Markup._getShadowHostIfPossible = function (node, depth) > > Nit: Remove the space between function and open parenthesis. Will do.
Ryosuke Niwa
Comment 5 2012-02-23 13:16:57 PST
Peter Kasting
Comment 6 2012-03-05 12:37:11 PST
*** Bug 78966 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.