WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
26333
alert during a dragenter event handler will crash the renderer
https://bugs.webkit.org/show_bug.cgi?id=26333
Summary
alert during a dragenter event handler will crash the renderer
Victor Wang
Reported
2009-06-11 14:58:11 PDT
The steps to reproduce the problem: 1. Go to the test URL above. 2. Drag the image. 3. Watch the renderer crash. The crux of the page is this: function doOnLoad() { var e = document.documentElement; var f = function() { alert('You is pwned.'); }; if (e.addEventListener) { e.addEventListener('dragenter', f, false); } else { e.attachEvent('ondragenter', f); } }
Attachments
Porpsed Fix for Bug 26333
(4.11 KB, patch)
2009-06-11 16:55 PDT
,
Victor Wang
eric
: review-
Details
Formatted Diff
Diff
Patch updated per comments
(4.23 KB, patch)
2009-06-12 10:56 PDT
,
Victor Wang
eric
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Victor Wang
Comment 1
2009-06-11 16:55:42 PDT
Created
attachment 31178
[details]
Porpsed Fix for
Bug 26333
Eric Seidel (no email)
Comment 2
2009-06-12 02:53:43 PDT
Comment on
attachment 31178
[details]
Porpsed Fix for
Bug 26333
You should set CHANGELOG_NAME in your environment. or REAL_NAME. prepare-ChangeLog isn't smart enough on windows to be able to get your real name automatically, hence: 1 2009-06-11 victorw <
victorw@chromium.org
> You should mention why this needs to be a manual test (because DRT doesn't show alerts) Officially our style guidelines don't use { } on single line ifs: + if (window.layoutTestController) { + layoutTestController.dumpAsText(); + } I'm not sure that our style guide is clear about that for JavaScript, so it might not matter. WK style is also to use more descriptive variable names: + var e = document.documentElement; + var f = function() { + alert('Click OK button.'); + }; at least in code. ;) Probably should say WebKit: + <p>Do the following and see if Chromium crashes.</p> Because that will crash Safari too, no? Otherwise looks fine. r- for the nits above.
Eric Seidel (no email)
Comment 3
2009-06-12 02:54:47 PDT
Comment on
attachment 31178
[details]
Porpsed Fix for
Bug 26333
Your SVN is also not configured correctly: Index: WebCore/manual-tests/resources/drag-image.png =================================================================== Cannot display: file marked as a binary type. svn:mime-type = application/octet-stream Property changes on: WebCore/manual-tests/resources/drag-image.png ___________________________________________________________________ Added: svn:executable + * Added: svn:mime-type + application/octet-stream It isn't correctly detecting the png type. Hence PrettyPatch isn't displaying the image inline in the bug. :)
Victor Wang
Comment 4
2009-06-12 10:56:51 PDT
Created
attachment 31202
[details]
Patch updated per comments
Eric Seidel (no email)
Comment 5
2009-06-12 12:12:03 PDT
Comment on
attachment 31202
[details]
Patch updated per comments Added: Added: svn:mine-type + image/png by accident it seems. :) I'm not sure what is confusing PrettyPatch. Windows SVN adds executable properties which it shouldn't. This looks fine. In the future you need to remember to mark your patches for review if you want them reviewed though. :)
http://webkit.org/coding/contributing.html
Victor Wang
Comment 6
2009-06-12 12:17:18 PDT
Sure. Was trying to figure out the png display issue before I send out request to review the new patch. All comments addressed. I don't have commit access, could you land my patch if it looks good to you? Thanks! (In reply to
comment #5
)
> (From update of
attachment 31202
[details]
[review]) > Added: > Added: svn:mine-type > + image/png > > by accident it seems. :) > > I'm not sure what is confusing PrettyPatch. > > Windows SVN adds executable properties which it shouldn't. > > This looks fine. In the future you need to remember to mark your patches for > review if you want them reviewed though. :) >
http://webkit.org/coding/contributing.html
>
Adam Barth
Comment 7
2009-06-13 11:11:28 PDT
I'll try to land this. Hopefully the PNG part will work.
Adam Barth
Comment 8
2009-06-13 14:35:38 PDT
Sending WebCore/ChangeLog Adding WebCore/manual-tests/drag-enter-alert.html Adding (bin) WebCore/manual-tests/resources/drag-image.png Sending WebCore/page/DragController.cpp Transmitting file data .... Committed revision 44659.
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