Reproductions steps (copied from http://code.google.com/p/chromium/issues/detail?id=97104): 1.Go to http://dojotoolkit.org/widgets 2.Click the Editor tab. 3.In the Enabled editor, add multiple lines to the end of the content so the scrollbar shows. 4.Press Ctrl+A to select all contents and then Ctrl+C and copy them into the clipboard. 5.Press Ctrl+V. http://crbug.com/97104
Created attachment 112398 [details] fixes the bug
Comment on attachment 112398 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=112398&action=review > Source/WebCore/ChangeLog:5 > + It would be nice to have a description of the issue and the relevant fix. It is not clear at all from the bug what is happening. Could you please provide more details? The patch seems to be adding handling of the CSSPropertyTextDecoration property, but I don't understand at all how this is related to the bug.
(In reply to comment #2) > (From update of attachment 112398 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=112398&action=review > > > Source/WebCore/ChangeLog:5 > > + > > It would be nice to have a description of the issue and the relevant fix. > It is not clear at all from the bug what is happening. Could you please provide more details? The patch seems to be adding handling of the CSSPropertyTextDecoration property, but I don't understand at all how this is related to the bug. Oops, apparently I forgot to add that. Will do in a minute.
Comment on attachment 112398 [details] fixes the bug Attachment 112398 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10211358 New failing tests: editing/pasteboard/5134759.html
Created attachment 112410 [details] Added change log entry
Comment on attachment 112410 [details] Added change log entry Attachment 112410 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10208498 New failing tests: editing/pasteboard/5134759.html
Ping reviewers.
Comment on attachment 112410 [details] Added change log entry View in context: https://bugs.webkit.org/attachment.cgi?id=112410&action=review > Source/WebCore/editing/markup.cpp:188 > + ASSERT(propertyMissingOrEqualToNone(style, CSSPropertyWebkitTextDecorationsInEffect)); It would be nice to have a comment about the new assert. > Source/WebCore/editing/markup.cpp:-526 > - return style && style->style() && !propertyMissingOrEqualToNone(style->style(), CSSPropertyTextDecoration); This has been removed because we don't expect to have this property anymore, correct? > LayoutTests/editing/pasteboard/19644-2-expected.txt:1 > +<span style="background-color: rgb(187, 187, 187); ">This tests to make sure that we wrap copied markup in a div to hold a fully selected body's attributes and style when the that body has a background-color. If you copy and paste this text into Mail, it should have a grey background.</span> Why is having a span now equivalent to the div before? > LayoutTests/platform/mac/editing/pasteboard/5134759-expected.txt:21 > + text run at (39,0) width 45: "World!" similar question: why is it ok not to have a div anymore?
Comment on attachment 112410 [details] Added change log entry View in context: https://bugs.webkit.org/attachment.cgi?id=112410&action=review >> Source/WebCore/editing/markup.cpp:188 >> + ASSERT(propertyMissingOrEqualToNone(style, CSSPropertyWebkitTextDecorationsInEffect)); > > It would be nice to have a comment about the new assert. How about "wrappingStyleForSerialization should have removed -webkit-text-decorations-in-effect"? >> Source/WebCore/editing/markup.cpp:-526 >> - return style && style->style() && !propertyMissingOrEqualToNone(style->style(), CSSPropertyTextDecoration); > > This has been removed because we don't expect to have this property anymore, correct? No, we expect it to be there but we'll include the effective value in wrappingStyleForSerialization instead of cloning all ancestors up to an element with text-decoration property.
Comment on attachment 112410 [details] Added change log entry View in context: https://bugs.webkit.org/attachment.cgi?id=112410&action=review >> LayoutTests/editing/pasteboard/19644-2-expected.txt:1 >> +<span style="background-color: rgb(187, 187, 187); ">This tests to make sure that we wrap copied markup in a div to hold a fully selected body's attributes and style when the that body has a background-color. If you copy and paste this text into Mail, it should have a grey background.</span> > > Why is having a span now equivalent to the div before? It's okay because this is the only content in body. This is a bit tricky case because div will change the background color of the entire block as supposed to span which changes the background color of the inline text. Even div doesn't occupy the entire body so it'll have a different background color below the div anyway. Also, I think most of users expect text color to be applied as inline style as done in TextEdit, Microsoft Word, etc..., not as a block style. >> LayoutTests/platform/mac/editing/pasteboard/5134759-expected.txt:21 >> + text run at (39,0) width 45: "World!" > > similar question: why is it ok not to have a div anymore? This was a redundant div. I guess I can clarify that in the change log. Or convert this to a dump-as-markup first so that the rebaseline is clearer.
Created attachment 112910 [details] Updated per comment
Comment on attachment 112910 [details] Updated per comment Attachment 112910 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10240158 New failing tests: editing/pasteboard/5134759.html
Comment on attachment 112910 [details] Updated per comment Looks good.
Comment on attachment 112910 [details] Updated per comment Clearing flags on attachment: 112910 Committed r98794: <http://trac.webkit.org/changeset/98794>
All reviewed patches have been landed. Closing bug.