[webkit-reviews] review denied: [Bug 27481] onbeforeunload not called at window close + frame or iframe focused : [Attachment 48156] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Feb 5 11:37:43 PST 2010


Darin Adler <darin at apple.com> has denied Charles Reis <creis at chromium.org>'s
request for review:
Bug 27481: onbeforeunload not called at window close + frame or iframe focused
https://bugs.webkit.org/show_bug.cgi?id=27481

Attachment 48156: Patch
https://bugs.webkit.org/attachment.cgi?id=48156&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
This looks like the correct fix. It is good to make Windows and Chromium match
the Mac in this respect.

> +	   Layout tests aren't able to test this bug, since it requires closing

> +	   the actual browser window, not calling window.close().

Did you try to make a test?

I understand that window.close() is not helpful to test this, but it should be
testable in DumpRenderTree. We have existing tests for beforeunload and they
had no problem with this. For example, onunload-clears-onbeforeunload.html.

If you do need a way to test this, we would typically add a way to do it to
DumpRenderTree. We have features already to test opening and closing multiple
windows. You can find 73 tests that use the
layoutTestController.setCanOpenWindows() and you can probably create a test
that way.

If you try all these things and fail, at least you should add a test to the
WebCore/manual-tests directory.

review- because this does need a test, and further, an automated test should be
possible


More information about the webkit-reviews mailing list