Summary: | REGRESSION: showModalDialog returnValue ignored, function result is always "undefined" | ||
---|---|---|---|
Product: | WebKit | Reporter: | Mark Malone <markmalone> |
Component: | DOM | Assignee: | Darin Adler <darin> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | andersca, darin, ggaren, markmalone, mjs |
Priority: | P1 | Keywords: | InRadar, Regression |
Version: | 420+ | ||
Hardware: | Mac | ||
OS: | OS X 10.4 | ||
URL: | http://groupaware.com/aspect/dialog/Launcher.html | ||
Attachments: |
Description
Mark Malone
2006-06-27 11:18:59 PDT
Works in 10.4.7. The problem is that Window::clear() is not being called at all. This is presumably having many bad symptoms, not just this one! Created attachment 9092 [details]
patch, including change log and a manual test
Not the most elegant solution to the problem, but does fix it.
Created attachment 9095 [details]
patch, including change log and a manual test
Comment on attachment 9095 [details]
patch, including change log and a manual test
r=me
Committed revision 15097. Comment on attachment 9095 [details]
patch, including change log and a manual test
Further testing show this not working in some cases. New patch forthcoming.
Created attachment 9269 [details]
better patch with change log, can still use the old test case
Created attachment 9271 [details]
better patch with change log, can still use the old test case
Comment on attachment 9271 [details]
better patch with change log, can still use the old test case
r=me
I think kjs_window.cpp could use a comment explaining why you need to do this:
+ if (!returnValue)
+ returnValue = dialogWindow->getDirect("returnValue");
Also, it looks like you changed the indenting in KJSProxy::clear from 4 spaces to 2.
(In reply to comment #12) > I think kjs_window.cpp could use a comment explaining why you need to do this: > > + if (!returnValue) > + returnValue = dialogWindow->getDirect("returnValue"); OK. Will do. > Also, it looks like you changed the indenting in KJSProxy::clear from 4 spaces > to 2. Actually, my previous fix changed the indenting in KJSProxy::clear from 2 spaces to 4, and I rolled it out. My new change was small enough that I did not feel comfortable reindenting the function at the same time. Committed revision 15239. |