RESOLVED FIXED 48799
Crash when dragging and dropping in a document with an invalid XHTML header
https://bugs.webkit.org/show_bug.cgi?id=48799
Summary Crash when dragging and dropping in a document with an invalid XHTML header
Daniel Cheng
Reported 2010-11-01 16:18:37 PDT
http://crbug.com/61182. Open the SVG, then try to drag and drop the XHTML file into the window displaying the SVG.
Attachments
Patch (1.37 KB, patch)
2010-11-01 16:25 PDT, Daniel Cheng
no flags
Patch (4.27 KB, patch)
2011-03-14 00:03 PDT, Hajime Morrita
tony: review+
Daniel Cheng
Comment 1 2010-11-01 16:25:55 PDT
Tony Chang
Comment 2 2010-11-01 16:40:18 PDT
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 3 2010-11-01 17:27:09 PDT
Comment on attachment 72592 [details] Patch R- while I work on layout test.
Alexey Proskuryakov
Comment 4 2010-11-01 23:09:36 PDT
See also: bug 48791.
Daniel Cheng
Comment 5 2010-11-05 16:09:36 PDT
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
Comment 6 2010-11-08 16:03:20 PST
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
Comment 7 2010-12-03 01:08:53 PST
Crash is P1.
Adele Peterson
Comment 8 2011-01-11 15:52:13 PST
*** Bug 48791 has been marked as a duplicate of this bug. ***
Adele Peterson
Comment 9 2011-03-08 17:43:18 PST
Any update on this? Daniel, are you still working on it?
Hajime Morrita
Comment 10 2011-03-14 00:03:11 PDT
Hajime Morrita
Comment 11 2011-03-14 00:05:32 PDT
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 12 2011-03-14 10:18:25 PDT
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
Comment 13 2011-03-14 18:22:51 PDT
(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
Comment 14 2011-03-14 19:39:09 PDT
Ryosuke Niwa
Comment 15 2011-03-14 22:12:56 PDT
David Kilzer (:ddkilzer)
Comment 16 2011-03-15 14:30:00 PDT
Note You need to log in before you can comment on or make changes to this bug.