Bug 64423 - Line-breaks are disappeared from textarea.value if user pastes a text into a no-wrap-ed textarea.
Summary: Line-breaks are disappeared from textarea.value if user pastes a text into a ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-12 19:22 PDT by Hajime Morrita
Modified: 2011-12-11 21:34 PST (History)
3 users (show)

See Also:


Attachments
Patch (7.26 KB, patch)
2011-07-12 19:24 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (7.10 KB, patch)
2011-07-27 04:27 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 2011-07-12 19:22:16 PDT
Originally reported at http://crbug.com/88736.
A fix will come shortly.
Comment 1 Hajime Morrita 2011-07-12 19:24:57 PDT
Created attachment 100608 [details]
Patch
Comment 2 Kent Tamura 2011-07-13 18:48:51 PDT
(In reply to comment #1)
> Created an attachment (id=100608) [details]
> Patch

I think your patch will fix the problem. But can we avoid to insert <div> or something into the inner text of <textarea> and <input>?
Comment 3 Ryosuke Niwa 2011-07-13 19:07:37 PDT
Comment on attachment 100608 [details]
Patch

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

> LayoutTests/editing/pasteboard/paste-textarea-wrap.html:36
> +description("This tests if pasted text preserves original line-breaks regardless their destination styles.");

Nit: regardless *of* their destination styles.
Comment 4 Ryosuke Niwa 2011-07-13 19:08:14 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > Created an attachment (id=100608) [details] [details]
> > Patch
> 
> I think your patch will fix the problem. But can we avoid to insert <div> or something into the inner text of <textarea> and <input>?

Yeah, we specifically avoid inserting div into the shadow DOM in various places.  We should figure out where this div is coming from and replace them with br's.
Comment 5 Hajime Morrita 2011-07-13 22:21:10 PDT
> > I think your patch will fix the problem. But can we avoid to insert <div> or something into the inner text of <textarea> and <input>?
> 
> Yeah, we specifically avoid inserting div into the shadow DOM in various places.  We should figure out where this div is coming from and replace them with br's.

Kent-san, Ryosuke, thanks for taking a look!
I'm not sure what should happen there. So I'll investigate how text insertion works at first.
Comment 6 Hajime Morrita 2011-07-27 04:27:09 PDT
Created attachment 102122 [details]
Patch
Comment 7 Hajime Morrita 2011-07-27 04:28:35 PDT
> > I think your patch will fix the problem. But can we avoid to insert <div> or something into the inner text of <textarea> and <input>?
> 
> Yeah, we specifically avoid inserting div into the shadow DOM in various places.  We should figure out where this div is coming from and replace them with br's.

I updated the patch to use <br>.
Niwa-san, could you  take a look?
Comment 8 Ryosuke Niwa 2011-07-31 14:06:07 PDT
Comment on attachment 102122 [details]
Patch

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

> Source/WebCore/editing/markup.cpp:772
> +                element = createHTMLElement(document, spanTag);
> +                fillContainerFromString(element.get(), s);
> +                fragment->appendChild(element.release(), ASSERT_NO_EXCEPTION);
> +                element = createBreakElement(document);
> +                fragment->appendChild(element.release(), ASSERT_NO_EXCEPTION);

This seems like a wrong layer to fix this.  I'd suspect where we need to fix is ReplaceSelectionCommand.
Comment 9 Hajime Morrita 2011-08-01 02:11:32 PDT
Comment on attachment 102122 [details]
Patch

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

>> Source/WebCore/editing/markup.cpp:772
>> +                fragment->appendChild(element.release(), ASSERT_NO_EXCEPTION);
> 
> This seems like a wrong layer to fix this.  I'd suspect where we need to fix is ReplaceSelectionCommand.

I don't think so.
ReplaceSelectionCommand just emits a textInput event,
And Editor handles it, then calls this method.
Actually this createFragmentFromText() has similar trick at the beginning of the function for preserveNewLine().

Hopefully we have FragmentBuilder to make these code more organized, like MarkupAccumulator does.
But that's another story.
Comment 10 Hajime Morrita 2011-08-23 02:20:41 PDT
Comment on attachment 102122 [details]
Patch

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

>>> Source/WebCore/editing/markup.cpp:772
>>> +                fragment->appendChild(element.release(), ASSERT_NO_EXCEPTION);
>> 
>> This seems like a wrong layer to fix this.  I'd suspect where we need to fix is ReplaceSelectionCommand.
> 
> I don't think so.
> ReplaceSelectionCommand just emits a textInput event,
> And Editor handles it, then calls this method.
> Actually this createFragmentFromText() has similar trick at the beginning of the function for preserveNewLine().
> 
> Hopefully we have FragmentBuilder to make these code more organized, like MarkupAccumulator does.
> But that's another story.

Actually, the fragment is build _before_ ReplaceSelectionCommand_ is instantiated, 
The created fragment is passed to ReplaceSelectionCommand constructor instead.
So there is nothing to do for ReplaceSelectionCommand.
Comment 11 Ryosuke Niwa 2011-08-23 10:16:57 PDT
Comment on attachment 102122 [details]
Patch

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

>>>> Source/WebCore/editing/markup.cpp:772
>>>> +                fragment->appendChild(element.release(), ASSERT_NO_EXCEPTION);
>>> 
>>> This seems like a wrong layer to fix this.  I'd suspect where we need to fix is ReplaceSelectionCommand.
>> 
>> I don't think so.
>> ReplaceSelectionCommand just emits a textInput event,
>> And Editor handles it, then calls this method.
>> Actually this createFragmentFromText() has similar trick at the beginning of the function for preserveNewLine().
>> 
>> Hopefully we have FragmentBuilder to make these code more organized, like MarkupAccumulator does.
>> But that's another story.
> 
> Actually, the fragment is build _before_ ReplaceSelectionCommand_ is instantiated, 
> The created fragment is passed to ReplaceSelectionCommand constructor instead.
> So there is nothing to do for ReplaceSelectionCommand.

What I'm saying that createFragment changing behavior depending on whether we're inside a shadow DOM or not seems wrong.
Comment 12 Ryosuke Niwa 2011-08-23 10:18:05 PDT
For example, are you sure that this bug doesn't reproduce in a contenteditable area that has the exactly same CSS style as the one in the shadow DOM of textarea?
Comment 13 Hajime Morrita 2011-12-11 21:28:29 PST
Comment on attachment 102122 [details]
Patch

Clearing r? for now.
Comment 14 Ryosuke Niwa 2011-12-11 21:33:57 PST
Oops, I had totally forgot about this bug and made a very similar change in http://trac.webkit.org/changeset/102392. Sorry about that :(