Bug 9254 – Quirksmode: Safari ignores window.close() after form submission

• Component: Forms
• Status: RESOLVED WONTFIX

[Adele Peterson]

http://www.quirksmode.org/bugreports/archives/safari/index.html

"Safari 2.0.2 (416.12) seems to be unable to process the javascript:window.close() function after a form submission. The first time into the test page, the 'close' button works as expected. However after you've used the submit button, it is no longer functional. The click event is firing, but the close request is not being honoured.

Test page http://www.evergreenplayers.org/cheddar/test/testclose.php
Workaround is not included
Reported by: ray berry."

[Joost de Valk (AlthA)]

I think i've seen this one before, but i can't find it. This needs reduction as the testcase is gone. Changing component.

[Joost de Valk (AlthA)]

adding keyword as per previous comment.

[Robert Blaut]

The test page is broken. Is the bug still alive?

[Robert Blaut]

Created attachment 26364 [details]
recovered test case from www.archive.org

I've recovered the test case, but in this state sending form doesn't work.

[Mark Hahnenberg]

I looked into this and it appears to be the fact that when calling the Page constructor, m_allowScriptsToCloseWindows is set to false, but initially in Page.h it is set to true.

[Mark Hahnenberg]

Sorry, all of this happens in Settings.h and Settings.cpp, not Page.h.

[Mark Hahnenberg]

Created attachment 101767 [details]
Modified test case

I changed the form action to be blank so that the form submission would still occur but wouldn't depend on any other external script.  Still seems to show the behavior described above.

[Mark Hahnenberg]

Created attachment 101777 [details]
Patch

[Mark Hahnenberg]

Another question we might want to ask ourselves is whether or not scripts should be allowed to call window.close() if they didn't call window.open().  Here's a brief summary of what the other major browsers do:

Firefox: disallows window.close() for scripts that didn't call window.open()
Chrome: same bug as in Safari, since they rely on WebCore
IE9: asks the user with a modal dialogue box if they want to allow the script to close the window
Opera 11.50: allows window.close()

[Alexey Proskuryakov]

Comment on attachment 101777 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=101777&action=review

Does the proposed behavior match HTML5, Firefox and IE?

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

We'll need them.

[Mark Hahnenberg]

> Does the proposed behavior match HTML5, Firefox and IE?
Since Firefox and IE don't match, then it's obvious that we can't match both.

As far as HTML5 goes, the following passage from http://www.w3.org/TR/html5/browsers.html#window is the most detailed that I can find about the behavior of window.close():

"The close() method on Window objects should, if the corresponding browsing context A is an auxiliary browsing context that was created by a script (as opposed to by an action of the user), and if the browsing context of the script that invokes the method is allowed to navigate the browsing context A, close the browsing context A (and may discard it too)."

This doesn't explicitly forbid closing the window if it's not an auxiliary browsing context, but it seems to suggest that we should forbid it.

This patch does the very opposite of this, so perhaps it's back to the drawing board?

[Mark Hahnenberg]

Created attachment 101793 [details]
More general test case

Btw, this issue is not specific to form submission.  This test case uses just plain links.

[Mark Hahnenberg]

As was pointed out to me, I shouldn't use the spec from which I quoted and should instead use something along the lines of http://www.whatwg.org/specs/web-apps/current-work/complete/.  In reference to the window.close() specification, they are identical in this instance, but I figured I'd correct myself anyways.

[Mark Hahnenberg]

Created attachment 101888 [details]
Patch

[WebKit Review Bot]

Comment on attachment 101888 [details]
Patch

Attachment 101888 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9249055

New failing tests:
fast/harness/show-modal-dialog.html
inspector/console/console-long-eval-crash.html
http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-window-open.html
fast/events/show-modal-dialog-onblur-onfocus.html
fast/frames/crash-removed-iframe.html

[Mark Hahnenberg]

Created attachment 102014 [details]
Patch

[Adam Barth]

Comment on attachment 102014 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=102014&action=review

> Source/WebCore/ChangeLog:17
> +        even when allowScriptsToCloseWindows was false.  This change should make us consistent
> +        with the HTML5 specification with respect to when scripts should be allowed to 
> +        call window.close().

Yay!

[WebKit Review Bot]

Comment on attachment 102014 [details]
Patch

Attachment 102014 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9251397

New failing tests:
fast/harness/show-modal-dialog.html
inspector/console/console-long-eval-crash.html
http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-window-open.html
fast/events/show-modal-dialog-onblur-onfocus.html
fast/frames/crash-removed-iframe.html

[Elliott Sprehn]

This never got landed due to failed tests? Is it worth fixing? What happened? :)

[Darin Adler]

I’d like to see this get fixed; not sure what the status of the patch is.

[Mark Hahnenberg]

I think the actual issue is there was a bit of code from back in the KJS/KHTML days where having a back-forward list of length > 1 would cause us to close the window, even if the window wasn't opened by a script (see http://trac.webkit.org/changeset/24028/trunk/WebCore/bindings/js/kjs_window.cpp).

Just removing this one condition makes the behavior consistent with that described in the comments above. Patch should follow shortly.

[Mark Hahnenberg]

(In reply to comment #21)
> I think the actual issue is there was a bit of code from back in the KJS/KHTML days where having a back-forward list of length > 1 would cause us to close the window, even if the window wasn't opened by a script (see http://trac.webkit.org/changeset/24028/trunk/WebCore/bindings/js/kjs_window.cpp).

Just to be clear, the code linked to wasn't removed--it was moved to DOMWindow::close in DOMWindow.cpp in that same patch where it has survived in various forms to this day.

[Darin Adler]

Comment on attachment 102014 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=102014&action=review

> Source/WebCore/page/DOMWindow.cpp:1821
> +        targetFrame->page()->settings()->setAllowScriptsToCloseWindows(true);

It does not seem quite right to change the settings at this level. Settings normally come from WebKit clients, and are not set by WebKit itself. Maybe there’s another way to accomplish the same thing.

[Mark Hahnenberg]

(In reply to comment #23)
> (From update of attachment 102014 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=102014&action=review
> 
> > Source/WebCore/page/DOMWindow.cpp:1821
> > +        targetFrame->page()->settings()->setAllowScriptsToCloseWindows(true);
> 
> It does not seem quite right to change the settings at this level. Settings normally come from WebKit clients, and are not set by WebKit itself. Maybe there’s another way to accomplish the same thing.

My initial approach was incorrect. I don't think this bug has to do with the allowScriptsToCloseWindows setting.

[Mark Hahnenberg]

> "The close() method on Window objects should, if the corresponding browsing context A is an auxiliary browsing context that was created by a script (as opposed to by an action of the user), and if the browsing context of the script that invokes the method is allowed to navigate the browsing context A, close the browsing context A (and may discard it too)."

Hmm, the spec has changed slightly to the following (according to http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#dom-window-close):

"The close() method on Window objects should, if the corresponding browsing context A is script-closable and the browsing context of the script that invokes the method is allowed to navigate the browsing context A, close the browsing context A.

A browsing context is script-closable if it is an auxiliary browsing context that was created by a script (as opposed to by an action of the user), or if it is a browsing context whose session history contains only one Document."

So according to the spec, our behavior is now considered correct B-)

[Mark Hahnenberg]

Based on my comments above, I'll close this as WONTFIX.