[webkit-reviews] review denied: [Bug 22301] dispatchDidFinishDocumentLoad and window.onload order are inconsistent : [Attachment 25585] New patch, this time with regression tests updated.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 5 09:54:24 PST 2008


Darin Adler <darin at apple.com> has denied Aaron Boodman <aa at chromium.org>'s
request for review:
Bug 22301: dispatchDidFinishDocumentLoad and window.onload order are
inconsistent
https://bugs.webkit.org/show_bug.cgi?id=22301

Attachment 25585: New patch, this time with regression tests updated.
https://bugs.webkit.org/attachment.cgi?id=25585&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Looks good.

> +	https://bugs.webkit.org/show_bug.cgi?id=22301
> +
> +	Make dispatchDidFinishLoading() always fire before
didFinishLoadForFrame().

Tabs in ChangeLog. This makes it harder for the committer to land your patch.

> Index: LayoutTests/fast/dom/Window/get-set-properties-expected.txt
> ===================================================================
> --- LayoutTests/fast/dom/Window/get-set-properties-expected.txt      
(revision 38838)
> +++ LayoutTests/fast/dom/Window/get-set-properties-expected.txt      
(working copy)
> @@ -1,4 +1,3 @@
> -main frame "window.marker.toString()" - has 1 onunload handler(s)
>  This page tests getting and setting window properties and functions.

Why is this test result correct? It looks to me like you broke something if the
onunload handler message isn't being logged any more.

review- because of this test result change that needs to be investigated.

If you can explain why the test result is OK, then it's fine to put this same
patch up for review again.


More information about the webkit-reviews mailing list