Always parse pasted fragments as HTML even on XHTML pages
Created attachment 169713 [details] Patch
Comment on attachment 169713 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169713&action=review > Source/WebCore/editing/markup.cpp:670 > + element->removeChild(child.get()); Is there a reason to unwrap the RefPtr here but pass it below? > Source/WebCore/editing/markup.cpp:681 > + if (node->hasTagName(htmlTag) || node->hasTagName(headTag) || node->hasTagName(bodyTag)) { What's the purpose of this method? I can just put <head><head></head></head> into the pasteboard and you'd leave the inner one behind.
Comment on attachment 169713 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169713&action=review >> Source/WebCore/editing/markup.cpp:670 >> + element->removeChild(child.get()); > > Is there a reason to unwrap the RefPtr here but pass it below? removeChild() takes a raw pointer, insertBefore() takes a PassRefPtr. So I'm using .get() as minimally as I can. >> Source/WebCore/editing/markup.cpp:681 >> + if (node->hasTagName(htmlTag) || node->hasTagName(headTag) || node->hasTagName(bodyTag)) { > > What's the purpose of this method? I can just put <head><head></head></head> into the pasteboard and you'd leave the inner one behind. Added a short comment, see if that helps.
Created attachment 169715 [details] Patch
Comment on attachment 169715 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169715&action=review > LayoutTests/ChangeLog:17 > + * platform/mac/editing/pasteboard/paste-xml-expected.txt: Now that we > + use the HTML parser, parsing the paste succeeds and so we insert DOM > + instead of plain text. Similar rebaselines may be needed on other > + platforms. maybe point to the pretty-diff bug to explain this diff?
Created attachment 169727 [details] Patch for landing
Patch for landing is greatly simplified, since it turns out the special code to handle <html>, <body>, and <head> is not needed at all (because we use the HTML5 fragment parsing algorithm with <body> as the context element, which means we ignore all three of those tags.
Comment on attachment 169727 [details] Patch for landing Waiting to cq+ so I can watch the waterfalls in case rebaselining is needed.
+ The Mac port previously worked around this problem by falling back to plain text + when parsing failed, but switching to HTML seems like a clear improvement. Are you aware of any way to reproduce this in Safari on Mac in practice? I briefly tried, but couldn't get in this code path.
(In reply to comment #9) > + The Mac port previously worked around this problem by falling back to plain text > + when parsing failed, but switching to HTML seems like a clear improvement. > > Are you aware of any way to reproduce this in Safari on Mac in practice? I briefly tried, but couldn't get in this code path. All you should need is an xhtml page with a contentEditable element in it. Try to copy some html and paste it into that element. Fwiw, The relevant code is in EditorMac.mm, Editor::documentFragment(). Under DRT, editing/pasteboard/paste-xml.xhtml walks through this "try to parse as XML, fail, fall back to text" case.
I tried that, and got styled text pasted. Possibly because there were additional flavors on the pasteboard, but this is why I'm asking for steps to reproduce.
Created attachment 169742 [details] Test case Note that the ".xhtml" suffix is necessary for Safari to treat this as an XHTML document. When I select the first line and paste it onto the second line, the bolding disappears.
Thank you, I can reproduce now. I experimented with a data:application/xhtml+xml URL, and looks like these do not make XHTML documents in Safari.
Comment on attachment 169727 [details] Patch for landing Rejecting attachment 169727 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ent): Merge conflict in Source/WebKit2/ChangeLog Failed to merge in the changes. Patch failed at 0001 [EFL] Make plugin process debugging easier (PLUGIN_PROCESS_COMMAND_PREFIX) When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. Full output: http://queues.webkit.org/results/14518146
Comment on attachment 169727 [details] Patch for landing Clearing flags on attachment: 169727 Committed r132211: <http://trac.webkit.org/changeset/132211>
All reviewed patches have been landed. Closing bug.