Bug 79305 - REGRESSION(r99076): WebKit pastes the trailing newline into a single-line text field
Summary: REGRESSION(r99076): WebKit pastes the trailing newline into a single-line tex...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: HasReduction
: 78966 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-02-22 16:53 PST by Ryosuke Niwa
Modified: 2012-03-05 12:37 PST (History)
6 users (show)

See Also:


Attachments
demo (336 bytes, text/html)
2012-02-22 16:53 PST, Ryosuke Niwa
no flags Details
Fixes the bug (6.01 KB, patch)
2012-02-22 17:37 PST, Ryosuke Niwa
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 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
Comment 2 Ryosuke Niwa 2012-02-22 17:37:10 PST
Created attachment 128348 [details]
Fixes the bug
Comment 3 Tony Chang 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.
Comment 4 Ryosuke Niwa 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.
Comment 5 Ryosuke Niwa 2012-02-23 13:16:57 PST
Committed r108668: <http://trac.webkit.org/changeset/108668>
Comment 6 Peter Kasting 2012-03-05 12:37:11 PST
*** Bug 78966 has been marked as a duplicate of this bug. ***