Bug 48799 – Crash when dragging and dropping in a document with an invalid XHTML header

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Daniel Cheng]

http://crbug.com/61182. Open the SVG, then try to drag and drop the XHTML file into the window displaying the SVG.

[Daniel Cheng]

Created attachment 72592 [details]
Patch

[Tony Chang]

Comment on attachment 72592 [details]
Patch

Can you write a layout test for this?  Maybe you can put the drag target in an iframe and drag link to the svg into the iframe?

[Daniel Cheng]

Comment on attachment 72592 [details]
Patch

R- while I work on layout test.

[Alexey Proskuryakov]

See also: bug 48791.

[Daniel Cheng]

Adding morrita to see what he thinks of the fix.

Several things I noticed when trying to write a layout test for this:
findEventTargetFrom() is being called with an empty selection. In Safari/Chrome, m_frame->document()->body() seems to be returning NULL which causes findEventTargetFrom() to return NULL. However, in DRT, m_frame->document()->body() is *not* NULL. I do not know why there is a difference, but as a result, I am unable to write a layout test to automatically trigger this bug.

I can add a manual test to this bug if you want, but I don't think I understand enough to figure out the behavior difference between DRT and an actual browser. Maybe the DRT doesn't have special handling for invalid XML parses?

[Tony Chang]

I did some testing, it looks like the svg file isn't rendered as svg, it's just parsed as XML.  So you can repro this bug by loading a random XML document without a body tag and then dragging anything into the XML document.

I'm not exactly sure why this doesn't repro in DRT.  It looks like DRT always loads random XML as an HTML document.  Maybe abarth has some insight into how we decide if something is an XML document vs an HTML document.

Here's an example: http://plexode.com/eval3/#ht=%3Ciframe%20src%3D%22data%3Atext%2Fxml%2C%3Cfoo%3E%3C%2Ffoo%3E%22%3E%3C%2Fiframe%3E&ohh=1&ohj=1&jt=&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1
If you drag anything into the iframe (e.g., a bookmark from the bookmark toolbar), we crash trying to reference body.  The check seems OK, but it seems better if we can verify that we have an HTML document.

[Ryosuke Niwa]

Crash is P1.

[Adele Peterson]

*** Bug 48791 has been marked as a duplicate of this bug. ***

[Adele Peterson]

Any update on this?  Daniel, are you still working on it?

[Hajime Morrita]

Created attachment 85652 [details]
Patch

[Hajime Morrita]

Hi Adele, Daniel, 
I'm sorry for passing over the discussion.
I attached a fix and test that reproduces the crash.
Reviews are welcome.

[Tony Chang]

Comment on attachment 85652 [details]
Patch

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

> LayoutTests/editing/pasteboard/drop-file-svg.html:23
> +<p>PASS unless crash</p>

Nit: Please make this a full sentence.  Also, please mention that this test requires DRT.

> Source/WebCore/page/DragController.cpp:398
> +    if (!m_page->dragCaretController()->isNone()) {
> +        if (!dispatchTextInputEventFor(innerFrame, dragData))

Nit: Can we just combine these into 1 if statement using && ?

[Hajime Morrita]

(In reply to comment #12)
> (From update of attachment 85652 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=85652&action=review
> 
> > LayoutTests/editing/pasteboard/drop-file-svg.html:23
> > +<p>PASS unless crash</p>
> 
> Nit: Please make this a full sentence.  Also, please mention that this test requires DRT.
> 
> > Source/WebCore/page/DragController.cpp:398
> > +    if (!m_page->dragCaretController()->isNone()) {
> > +        if (!dispatchTextInputEventFor(innerFrame, dragData))
> 
> Nit: Can we just combine these into 1 if statement using && ?

Hi Tony,
thank you for your quick review! 
I'll fix these before landing.

[Hajime Morrita]

Committed r81101: <http://trac.webkit.org/changeset/81101>

[Ryosuke Niwa]

It seems like the test has been failing on Windows 7 since it was added:
http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r81105%20(10397)/editing/pasteboard/drop-file-svg-pretty-diff.html

[David Kilzer (:ddkilzer)]

<rdar://problem/8616986>