502156 - nsHTMLEditor::CreateDOMFragmentFromPaste should use CreateContextualFragment

Status: NEW

(Core :: DOM: Editor, defect, P5)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Currently it instantiates an nsIParser directly. It can just use nsContentUtils::CreateContextualFragment. This saves code and means that if the HTML5 parser is enabled, we'll use it. This lets us do cool stuff like copy/paste and drag/drop SVG-in-HTML.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Ignore the media test junk at the start of the patch.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Ignore the stray nsHTML5Module include as well.

Comment 3

[Peter Van der Beken [:peterv]]

Comment on attachment 386682 [details] [diff] [review]
fix

>diff --git a/editor/libeditor/html/nsHTMLDataTransfer.cpp b/editor/libeditor/html/nsHTMLDataTransfer.cpp

>@@ -2540,54 +2541,55 @@ nsresult nsHTMLEditor::CreateDOMFragment

>   nsCOMPtr<nsIDOMDocument> domDoc;
>   GetDocument(getter_AddRefs(domDoc));
> 
>   nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);
>   NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE);

I think you can now also remove these.

>   nsCOMPtr<nsIDOMDocumentFragment> contextfrag;

contextfrag looks unused too.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I checked this in:
http://hg.mozilla.org/mozilla-central/rev/4c139bbb4895
but it seemed to cause test failure:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1247199195.1247205665.8856.gz#err0
so I backed it out.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It seems test_bug478725.html fails because the current parser, when called via CreateContextualFragment, parses "<dl><dd>Hello Kitty</dd></dl>" into a document fragment that starts with a text node containing a single space. Similar for test_bug480972.html. The tests pass using the HTML5 parser.

I also noticed that although it's not picked up in the tests, copying and pasting plain text not including a node does not work with this patch and the old parser. It does work with the HTML5 parser.

I'm not sure how to proceed.

Comment 6

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, >=5 years untouched DOM/Storage bugs' priority.

If you have reason to believe this is wrong (especially for the severity), please write a comment and ni :jstutte.