superreview requested: [Bug 241972] new window opened by window.open('xxx.exe') or target="_blank" isn't closed automatically when a download begins : [Attachment 227973] tweaks in response to reviews

Marria Nazif <marria@gmail.com> has asked Darin Fisher (Google)
<darin@meer.net> for superreview:
Bug 241972: new window opened by window.open('xxx.exe') or target="_blank"
isn't closed automatically when a download begins
https://bugzilla.mozilla.org/show_bug.cgi?id=241972

Attachment 227973: tweaks in response to reviews
https://bugzilla.mozilla.org/attachment.cgi?id=227973&action=edit

------- Additional Comments from Marria Nazif <marria@gmail.com>
(In reply to comment #60)
> (From update of attachment 227564 [edit])
> So this window is only closed when the user chooses an action, not as soon as

> the helper dialog appears?

Correct.  I figured that one is no better than the other.  Also, I believe the
refresh header detection happens later as the code is now, so it's easier to
make the window closing code later too.

> docshell/base/nsDocShell.cpp
>	   // Transfer the load to the target DocShell...  Pass nsnull as the
>	   // window target name from to prevent recursive retargeting!
>	   //
>	   if (NS_SUCCEEDED(rv) && targetDocShell) {
> +	       aFlags |= INTERNAL_LOAD_FLAGS_NEW_WINDOW;
> 
> Hm... isn't this case also hit if the load gets targeted to an existing
window?
> 

Yea, I think you are right, moved this up to where we detect that a new window
needs to be opened.

> 
> webshell/public/nsIRefreshURI.idl
> \ No newline at end of file
> 
> please add one :)
> 

added :-)

(In reply to comment #59)
> (From update of attachment 227564 [edit])
> >Index: docshell/base/nsDocShell.cpp
> 
> >	    if (!refreshHeader.IsEmpty()) {
> >		SetupReferrerFromChannel(aChannel);
> >		rv = SetupRefreshURIFromHeader(mCurrentURI, refreshHeader);
> >+		if (NS_SUCCEEDED(rv)) {
> >+		  return NS_REFRESHURI_HEADER_FOUND;
> >+		}
> >	    }
> >	}
> 
> whoops... 4 space indentation
> 
> 
> >+	if (aIsNewWindowTarget) {
> >+	  nsCOMPtr<nsIWritablePropertyBag2> props = do_QueryInterface(channel);

> >+	  if (props) {
> >+	   
props->SetPropertyAsBool(NS_LITERAL_STRING("docshell.newWindowTarget"),
> >+				     PR_TRUE);
> >+	  }
> >+	}
> 
> ditto 

done and done.
0
bugzilla
7/3/2006 8:41:48 PM
mozilla.dev.super-review 29307 articles. 3 followers. Post Follow

0 Replies
1148 Views

Similar Articles

[PageSpeed] 24

Reply: