RESOLVED FIXED Bug 10837
REGRESSION: Yahoo New Charts Crashes WebKit. 
https://bugs.webkit.org/show_bug.cgi?id=10837
Summary REGRESSION: Yahoo New Charts Crashes WebKit.
David Scheck
Reported 2006-09-13 06:30:26 PDT
The URL works in Safari but not Webkit. WebKit-SVN-r16342.dmg
Attachments
Crash log (23.60 KB, text/plain)
2006-09-13 06:36 PDT, Mark Rowe (bdash) 		no flags
Details
Reduction (will assert or crash) (313 bytes, text/html)
2006-09-13 09:05 PDT, mitz 		no flags
Details
Proposed patch (11.25 KB, patch)
2006-09-20 23:11 PDT, Andrew Wellington 		andrew: review-
DetailsFormatted DiffDiff
Proposed patch without the tabs (11.29 KB, patch)
2006-09-20 23:26 PDT, Andrew Wellington 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Rowe (bdash)
Comment 1 2006-09-13 06:36:12 PDT
Confirmed with r16342.
Mark Rowe (bdash)
Comment 2 2006-09-13 06:36:35 PDT
Created attachment 10524 [details] Crash log
mitz
Comment 3 2006-09-13 08:19:48 PDT
ASSERTION FAILED: p (WebCore/bindings/js/kjs_window.cpp:378 static KJS::JSValue* KJS::Window::retrieve(WebCore::Frame*))
mitz
Comment 4 2006-09-13 09:05:05 PDT
Created attachment 10530 [details] Reduction (will assert or crash) The crash happens when trying to access the location property of a document that is no longer associated with a frame. I think the document's URL can be a good fallback in this case.
mitz
Comment 5 2006-09-20 22:27:21 PDT
*** Bug 10955 has been marked as a duplicate of this bug. ***
Andrew Wellington
Comment 6 2006-09-20 23:11:44 PDT
Created attachment 10677 [details] Proposed patch Patch to return null in line with Firefox
Andrew Wellington
Comment 7 2006-09-20 23:25:28 PDT
Comment on attachment 10677 [details] Proposed patch Oi! Who put those tabs in there... :-/
Andrew Wellington
Comment 8 2006-09-20 23:26:05 PDT
Created attachment 10678 [details] Proposed patch without the tabs
Eric Seidel (no email)
Comment 9 2006-09-21 02:31:05 PDT
Comment on attachment 10678 [details] Proposed patch without the tabs Looks good to me. Too bad this logic can't be put in the DOM c++ implementation class. That would sure make later auto-generation of JSHTMLDocument easier. Curious. I wonder if this is broken in the Obj-C version of this wrapper? Otherwise it looks fine. r=me.
Alexey Proskuryakov
Comment 10 2006-09-21 02:57:46 PDT
> Too bad this logic can't be put in the DOM c++ implementation class. Cannot the check be inside retrieveWindow() itself?
Darin Adler
Comment 11 2006-10-06 07:35:28 PDT
(In reply to comment #10) > > Too bad this logic can't be put in the DOM c++ implementation class. > > Cannot the check be inside retrieveWindow() itself? Yes. retrieveWindow should return 0 in this case. And we can change the jsUndefined() to jsNull(). We should make that change instead of this one.
Darin Adler
Comment 12 2006-10-06 07:37:14 PDT
(In reply to comment #9) > (From update of attachment 10678 [details] [edit]) > Too bad this logic can't be put in the DOM C++ implementation class. This logic *can* be put into the DOM C++ implementation class. I'm going to do that.
Darin Adler
Comment 13 2006-10-06 22:05:30 PDT
(In reply to comment #12) > (In reply to comment #9) > > (From update of attachment 10678 [details] [edit] [edit]) > > Too bad this logic can't be put in the DOM C++ implementation class. > > This logic *can* be put into the DOM C++ implementation class. I'm going to do > that. Actually, it can't because there is no C++ implementation for this method yet. So the patch is fine as-is. Also, I think the frame nil check is a good solution -- no need to change Window::retrieve just to fix this, although we could consider making that change later.
Mark Rowe (bdash)
Comment 14 2006-10-07 05:43:50 PDT
Landed in r16881.
Note You need to log in before you can comment on or make changes to this bug.