Bug 61683

Created attachment 95251 [details] Patch

(In reply to comment #1) > Created an attachment (id=95251) [details] > Patch Note: When I was working on this patch, I noticed that the behavior is no longer what the test describes ("DragMe" gets inserted after "DropAboveMe", not before). The behavior changed in changeset 24255. I updated the description, but is this test still valid? If not, I can delete it instead of updating it.

Comment on attachment 95251 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95251&action=review (In reply to comment #2) > Note: When I was working on this patch, I noticed that the behavior is no longer what the test describes ("DragMe" gets inserted after "DropAboveMe", not before). The behavior changed in changeset 24255. I updated the description, but is this test still valid? If not, I can delete it instead of updating it. I don't think we should remove this test since it's one of very few tests that test drag & drop. Also, would you mind renaming the file to something more descriptive? You can still include <rdar://4861080> in the test description. > LayoutTests/editing/pasteboard/4861080-expected.txt:1 > +This tests dropping content into the unwanted space above a list items content when the content is wrapped in a span. You should see 'DropAboveMe DragMe'. There is no longer unwanted space so we should rephrase this. > LayoutTests/editing/pasteboard/4861080.html:47 > eventSender.mouseUp(); The statements that compute dropx and dropy seem wrong. We should get rid of li.offsetParent.offset*.

Created attachment 95490 [details] Patch

Comment on attachment 95251 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95251&action=review >> LayoutTests/editing/pasteboard/4861080-expected.txt:1 >> +This tests dropping content into the unwanted space above a list items content when the content is wrapped in a span. You should see 'DropAboveMe DragMe'. > > There is no longer unwanted space so we should rephrase this. Done. >> LayoutTests/editing/pasteboard/4861080.html:47 >> eventSender.mouseUp(); > > The statements that compute dropx and dropy seem wrong. We should get rid of li.offsetParent.offset*. Done.

Comment on attachment 95490 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95490&action=review > LayoutTests/editing/pasteboard/drag-drop-list.html:50 > + Markup.description("This tests dropping content onto a list items content when the content is wrapped in a span. You should see 'DropAboveMe DragMe'. See <rdar://4861080>."); You should probably mention that the expected behavior is different from what's described in 4861080 because the rendering bug 4907469 has been fixed.

Created attachment 95497 [details] Patch

Comment on attachment 95497 [details] Patch Clearing flags on attachment: 95497 Committed r87778: <http://trac.webkit.org/changeset/87778>

All reviewed patches have been landed. Closing bug.

--- LayoutTests/editing/pasteboard/drag-drop-list-expected.txt (revision 0) +++ LayoutTests/editing/pasteboard/drag-drop-list-expected.txt (revision 0) @@ -0,0 +1,18 @@ +This tests dropping content onto a list items content when the content is [...] +| <div> +| id="contents" +| style="border: 1px solid red;" +| "DropAboveMe" +| "<#selection-anchor>Â DragMe<#selection-focus>" [...] Just one question about this part in the expected file: Why is this "Â " present before "DragMe"? Is it just a typo or is there a reason for it? Asking because at the moment that's the cause the test is failing in GTK and SnowLeopardBots. Some links showing the issue: http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20%28WebKit2%20Tests%29/r87787%20%2812181%29/editing/pasteboard/drag-drop-list-pretty-diff.html http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20%28WebKit2%20Tests%29/r87782%20%28342%29/editing/pasteboard/drag-drop-list-pretty-diff.html http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r87787%20%2822961%29/editing/pasteboard/drag-drop-list-pretty-diff.html http://build.webkit.org/results/GTK%20Linux%2032-bit%20Release/r87787%20%2814261%29/editing/pasteboard/drag-drop-list-pretty-diff.html http://build.webkit.org/results/GTK%20Linux%2032-bit%20Debug/r87781%20%2815653%29/editing/pasteboard/drag-drop-list-pretty-diff.html If it's a typo, please update the expected file when possible. Thanks!

(In reply to comment #10) > --- LayoutTests/editing/pasteboard/drag-drop-list-expected.txt (revision 0) > +++ LayoutTests/editing/pasteboard/drag-drop-list-expected.txt (revision 0) > @@ -0,0 +1,18 @@ > +This tests dropping content onto a list items content when the content is > [...] > +| <div> > +| id="contents" > +| style="border: 1px solid red;" > +| "DropAboveMe" > +| "<#selection-anchor>Â DragMe<#selection-focus>" > [...] > > Just one question about this part in the expected file: > > Why is this "Â " present before "DragMe"? Is it just a typo or is there a reason for it? I think it's a newline. I'm looking into it now. > Asking because at the moment that's the cause the test is failing in GTK and SnowLeopardBots. Yeah, there may be platform-dependent behavior with the newlines. I am working on figuring out the issue now. Thanks for the links!

Re-opening this bug because of the failing webkit2 and gtk tests.

(In reply to comment #10) > --- LayoutTests/editing/pasteboard/drag-drop-list-expected.txt (revision 0) > +++ LayoutTests/editing/pasteboard/drag-drop-list-expected.txt (revision 0) > @@ -0,0 +1,18 @@ > +This tests dropping content onto a list items content when the content is > [...] > +| <div> > +| id="contents" > +| style="border: 1px solid red;" > +| "DropAboveMe" > +| "<#selection-anchor>Â DragMe<#selection-focus>" > [...] > > Just one question about this part in the expected file: > > Why is this "Â " present before "DragMe"? Is it just a typo or is there a reason for it? This is smart paste. "Â " is "&nbsp; " automatically inserted by WebKit.

Committed in http://trac.webkit.org/changeset/87778.