Bug 11346 – _blank is assigned to Window name

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Madhu M]

If there is no name given in window.open, then in WebKit '_blank' is assigned to the name of the window. In Firefox it will be empty string. In WebKit if id is given and there is no name then name of the window is assigned as the id.
In firefox if the name is given as '_blank' it will be changed to "".

[Madhu M]

Created attachment 11132 [details]
Patch for review

It is a sample fix for review for the issue to make the behavior similar to Mozilla  Firefox

[Alexey Proskuryakov]

To ask for a review, please select '?' from a popup on the screen that opens when you click "Edit" link. A proper patch must include an automated testcase, but you can ask for a review just to get some feedback on the approach.

FWIW, when I enter "javascript:window.open()" in the address bar of ToT, I get a window named "about:/undefined".

[Madhu M]

Comment on attachment 11132 [details]
Patch for review

In window.open, if there is no name is given, as the second parameter, in Safari _blank is assigned to the name of the Window. But in Firefox it will be "".

[Madhu M]

Created attachment 11200 [details]
the zip contains the sample pages to show the bug

Open the main.htm in Safari and click on the link, it will give self.name=_blank.
Do the same in Firefox. It will be 'self.name= '

[Madhu M]

Created attachment 11202 [details]
Patch for the issue

In this patch I have modified the code, to make the behaviour of Safari similar to Firefox. I have assigned "" to Window name if the name is undefined or null. If the name is given as _blank it will be changed to ""

[Madhu M]

Comment on attachment 11202 [details]
Patch for the issue

Please review and give suggestion

[Maciej Stachowiak]

Reopened since this is not resolved.

[Maciej Stachowiak]

Comment on attachment 11202 [details]
Patch for the issue

My comments:

1) This patch appears to be in RTF format, it needs to be in plain text.

2) There is no ChangeLog entry.

3) There is no test case in the patch - this probably can't be tested automatically but it should include a test for the manual-tests directory.

4) This fix is at the wrong layer. There are ther constructs besides window.open that can open a new window, such as <a href="http://somesite.com/" target="_blank">, these need to be covered as well. The right place to fix this is in the Frame code itself - a request to open a new window for _blank should make sure that the name ends up empt instead of "_blank".

5) Comments in the code should not talk about making a specific fix, nor should they describe what the code is doing if that is obvious just from reading.

6) Coding style issue, there should be a space between the "if" and the open paren: if(frameName == "_blank")

Please addresse these issues and resubmit.

[Madhu M]

Created attachment 11964 [details]
The attachment is a fix for the issue

Reviewer, Please send me a sample change log. The attachment contains the fix as suggested by MJS

[David Kilzer (:ddkilzer)]

Comment on attachment 11964 [details]
The attachment is a fix for the issue

Thanks for the patch!  There are still some issues that need to be cleaned up, though.

1. Please use WebKitTools/Scripts/prepare-ChangeLog to generate your own ChangeLog entry, then use WebKitTools/Scripts/svn-create-patch to generate the patch.  More info here: http://webkit.org/coding/contributing.html

2. No tabs are allowed in the source code.  Please use spaces instead.  More info here:  http://webkit.org/coding/coding-style.html

3. A manual test in WebCore/manual-tests should be included with the patch per Maciej in Comment #8.

Almost there!

[David Kilzer (:ddkilzer)]

This may be fixed now. Tested local debug build of WebKit r19341 with Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8N1037).

[Andrew Wellington]

As ddkilzer mentioned this issue has been fixed (see FrameTree::uniqueChildName) in r13257