Bug 6550 – REGRESSION: window.open('myurl', '_self') fails to load myurl in current window

• Status: VERIFIED
• Resolution: FIXED
• Component: DOM

[Micah Laaker]

When clicking the "[ Empty ]" link next to either "Bulk" or "Trash" in Yahoo! Mail (to empty the entire 
folder), a dialog box appears saying:

"http://us.f323.mail.yahoo.com
Are you sure? Deleted Bulk messages are gone forever, and will not go to your Trash folder."

However, upon pressing the "OK" button, no action is executed, and no messages are deleted. Instead, 
a user must select the messages and press the "Delete" button (once inside either the "Bulk" or "Trash" 
folders). 

Current public build of Safari, as well as Firefox, perform this action as intended, but the past couple 
weeks of WebKit builds have not been working in this regard.

[Geoffrey Garen]

P1 because it's a regression.

[David Kilzer (:ddkilzer)]

Using nightly build r12295 I noticed a couple more things:

- The "Show" link next to "My Folders" doesn't work either.
- The JavaScript Console (from the Debug menu) doesn't show any errors for either the "Show" or the "Empty" links.

Confirmed this is a bug; changing status to NEW.

[David Kilzer (:ddkilzer)]

Doing a binary search on nightly builds, I've narrowed down the commit to between these two builds:

Empty link does not work: WebKit-CVS-2005-12-27 07:05:12 GMT.dmg
Empty link does work:     WebKit-CVS-2005-12-25 00:53:06 GMT.dmg

Using "svn log -r{YYYY-MM-DD}" and adjusting for GMT from my local timezone, that puts the suspect revisions in this range, inclusive:  r11760-r11772.

I will do more research when I have time later, but we should probably find a reduced test case first to narrow down the search for changes in the above revisions.

[David Kilzer (:ddkilzer)]

Oops...make that revisions r11759-r11772 inclusive.

[David Kilzer (:ddkilzer)]

I looked through the revisions, but nothing jumped out at me as an obvious fix.  There were a couple of large refactorings in there, which may or may not be the source of the issue.

Looking at the "Show" link (which changes to "Hide" once clicked and the page reloads) in Firefox's DOM Inspector, the "onclick" event to the <span id="personalfoldersdisplaytoggle">Show</span> tag is set to a function named PersonalFoldersDisplayToggle_Click():

function PersonalFoldersDisplayToggle_Click() {
    var sDoneURL = (g_oDoneURL) ? g_sDoneURL : document.URL;
    window.open("/" + g_sYMURI + "/Welcome?pers=1&.done=" + escape(sDoneURL) + "&" + g_sURLExtras, "_self");
}

This function seems to use window.open(..., "_self") to open a new URL, which jogs my memory regarding some of the refactoring done in the revisions listed.

The "Empty" link is an anchor tag like this:  <a href="javascript:EmptyFolder('@B@Bulk')">Empty</a>.  It also uses window.open(..., "_self") to open a new URL.

function EmptyFolder(folder)
    {
	var warnonempty = folder == "@B@Bulk" ? (!noDelBulkMsgWarning) : (!noDelAllMsgWarning)	
	var emptyFol = true;
	if(warnonempty)
	{
	    var sMessage = folder == "@B@Bulk" ? "Are you sure? Deleted Bulk messages are gone forever, and will not go to your Trash folder." : "Are you sure you want to empty the folder?";
	    emptyFol = showEmptyFolderWarning(sMessage, folder);
	}
	
	if(emptyFol)
   	{		
    	    var url = "/ym/ShowFolder?";
    	    url += folder == "Trash" ? "ET" : "EB";
    	    url += "=1&.crumb=Jh5hwHIvCG6&reset=1&YY=43036";
    	    window.open(url, '_self');
	}
    }

Looks like we may have a smoking gun!

[David Kilzer (:ddkilzer)]

The window.open("myurl", "_self") JavaScript function is broken.  Updated Summary field to reflect the actual bug.

[David Kilzer (:ddkilzer)]

Created attachment 5862 [details]
Reduced test case (should go back to bug page)

[Alice Liu]

<rdar://problem/4424132>

[Darin Adler]

I see two things right away:

    1) pop-up blocking is blocking this window.open call
    2) if I turn off pop-up blocking, this line of code stops it from working:

      if (frameName == "_top" || frameName == "_parent" || frameName == "_self")
          return jsUndefined();

Strange.

[Darin Adler]

My thinking is that issue (1) is just a duplicate of bug 3308. And as far as I can tell, issue (2) can be fixed by just removing what seems to be a bogus check. Doing some spelunking to find out where that code came from.

[Darin Adler]

I see now what caused this. It's easy to fix.

[Darin Adler]

Created attachment 6067 [details]
patch to fix bug, including a test

[David Kilzer (:ddkilzer)]

Verified fixed on Attachment 5862 [details] and on mail.yahoo.com.

[Eric Seidel (no email)]

Removing Regression keyword from bugs already fixed.