Bug 11346 - _blank is assigned to Window name
Summary: _blank is assigned to Window name
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-18 05:09 PDT by Madhu M
Modified: 2007-09-30 03:30 PDT (History)
1 user (show)

See Also:


Attachments
Patch for review (1.07 KB, text/plain)
2006-10-18 05:18 PDT, Madhu M
no flags Details
the zip contains the sample pages to show the bug (1.42 KB, application/x-zip-compressed)
2006-10-25 04:18 PDT, Madhu M
no flags Details
Patch for the issue (1.41 KB, patch)
2006-10-25 04:27 PDT, Madhu M
mjs: review-
Details | Formatted Diff | Diff
The attachment is a fix for the issue (337 bytes, patch)
2006-12-22 06:30 PST, Madhu M
ddkilzer: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Madhu M 2006-10-18 05:09:07 PDT
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 "".
Comment 1 Madhu M 2006-10-18 05:18:17 PDT
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
Comment 2 Alexey Proskuryakov 2006-10-18 09:24:02 PDT
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".
Comment 3 Madhu M 2006-10-25 04:13:38 PDT
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 "".
Comment 4 Madhu M 2006-10-25 04:18:14 PDT
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= '
Comment 5 Madhu M 2006-10-25 04:27:37 PDT
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 ""
Comment 6 Madhu M 2006-10-25 04:31:30 PDT
Comment on attachment 11202 [details]
Patch for the issue

Please review and give suggestion
Comment 7 Maciej Stachowiak 2006-10-31 04:23:36 PST
Reopened since this is not resolved.
Comment 8 Maciej Stachowiak 2006-10-31 10:07:38 PST
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.
Comment 9 Madhu M 2006-12-22 06:30:58 PST
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
Comment 10 David Kilzer (:ddkilzer) 2006-12-22 06:49:35 PST
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!
Comment 11 David Kilzer (:ddkilzer) 2007-02-01 13:05:45 PST
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).

Comment 12 Andrew Wellington 2007-09-30 03:30:28 PDT
As ddkilzer mentioned this issue has been fixed (see FrameTree::uniqueChildName) in r13257