Bug 48793 - Dragging and dropping into an empty document crashes WebKit
Summary: Dragging and dropping into an empty document crashes WebKit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P1 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-01 15:55 PDT by Daniel Cheng
Modified: 2010-12-03 11:25 PST (History)
3 users (show)

See Also:


Attachments
Patch (2.39 KB, patch)
2010-11-01 15:59 PDT, Daniel Cheng
no flags Details | Formatted Diff | Diff
Patch (3.26 KB, patch)
2010-11-08 16:23 PST, Daniel Cheng
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Cheng 2010-11-01 15:55:12 PDT
elementUnderMouse() in DragController.cpp can return null if the document is empty (see test.xhtml attached to crbug.com/61182).
Comment 1 Daniel Cheng 2010-11-01 15:59:57 PDT
Created attachment 72580 [details]
Patch
Comment 2 Tony Chang 2010-11-01 16:20:35 PDT
Comment on attachment 72580 [details]
Patch

Can you make a layout test for this?
Comment 3 Daniel Cheng 2010-11-01 17:26:58 PDT
Comment on attachment 72580 [details]
Patch

R- while I work on layout test.
Comment 4 Daniel Cheng 2010-11-05 16:17:37 PDT
This one seems to require an empty top-level document to repro. Given the current testing infrastructure, I do not think it is possible to write a layout test. I can attach a manual test to the bug if you want.
Comment 5 Tony Chang 2010-11-08 13:34:28 PST
Sure, let's just go for a manual test.
Comment 6 Daniel Cheng 2010-11-08 16:23:19 PST
Created attachment 73304 [details]
Patch
Comment 7 Tony Chang 2010-11-08 16:33:09 PST
Comment on attachment 73304 [details]
Patch

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

> WebCore/ChangeLog:13
> +        * manual-tests/drop-in-empty-doc2.xhtml: Added.

drop-in-empty-doc2.xhtml should go in manual-tests/resources/.

> WebCore/page/DragController.cpp:271
> +// This can return null if an empty document is loaded.

Is this new or have we had this bug for a long time?  It seems odd that this is suddenly not working.  Do you know which change regressed this?

I'm worried we're papering over the crash when the fix is elsewhere.
Comment 8 Daniel Cheng 2010-11-08 18:09:29 PST
(In reply to comment #7)
> (From update of attachment 73304 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=73304&action=review
> 
> > WebCore/ChangeLog:13
> > +        * manual-tests/drop-in-empty-doc2.xhtml: Added.
> 
> drop-in-empty-doc2.xhtml should go in manual-tests/resources/.
> 
> > WebCore/page/DragController.cpp:271
> > +// This can return null if an empty document is loaded.
> 
> Is this new or have we had this bug for a long time?  It seems odd that this is suddenly not working.  Do you know which change regressed this?
> 
> I'm worried we're papering over the crash when the fix is elsewhere.

I think we've had this bug for a long time (it crashes Chrome 7 as well).
Comment 9 Ryosuke Niwa 2010-12-03 01:08:33 PST
Crash is P1.
Comment 10 Tony Chang 2010-12-03 09:53:04 PST
Comment on attachment 73304 [details]
Patch

Sorry, I forgot about this.  I'm going to do some testing in Safari before landing, so removing cq flag.
Comment 11 Tony Chang 2010-12-03 11:18:27 PST
Committed r73276: <http://trac.webkit.org/changeset/73276>
Comment 12 Tony Chang 2010-12-03 11:25:46 PST
(In reply to comment #11)
> Committed r73276: <http://trac.webkit.org/changeset/73276>

I verified that this crashes on both Safari 5.0.x (mac) and Safari with WebKit ToT.  The patch fixes it.  I also moved the .xhtml test file into the resources subdir.