WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(4.27 KB, patch)
2011-03-14 00:03 PDT
,
Hajime Morrita
tony
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Daniel Cheng
Comment 1
2010-11-01 16:25:55 PDT
Created
attachment 72592
[details]
Patch
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
Created
attachment 85652
[details]
Patch
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
Committed
r81101
: <
http://trac.webkit.org/changeset/81101
>
Ryosuke Niwa
Comment 15
2011-03-14 22:12:56 PDT
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)
Comment 16
2011-03-15 14:30:00 PDT
<
rdar://problem/8616986
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug