319257 - URLs from external apps don't bring window to front

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Chris Lawson (gone)]

If the Download Manager is the frontmost application window, loading a URL from an external app will no longer bring the frontmost *browser* window to the foreground.

This was caused by the checkin for bug 295858. Oops.

Patch coming.

Comment 1

[Chris Lawson (gone)]

Attachment: Get the new BWC, then activate its view properly

Comment 2

[Chris Lawson (gone)]

Requesting blocking for 1.0.

Sorry for the stupidity here. I should've caught it in testing yesterday.

cl

Comment 3

[Stuart Morgan]

Comment on attachment 205110 [details] [diff] [review]
Get the new BWC, then activate its view properly

Please do this the way I suggest in the previous bug, setting controller  to the frontmost window in the openNewWindow case (and using the original activate line).  There's no reason to do a somewhat expensive operation to get the front controller again in all the cases where it's known to be the same.

Comment 4

[Chris Lawson (gone)]

Attachment: Only get the new BWC in the "new window" case

Thanks for your patience, Stuart. How's that one?

cl

Comment 5

[Chris Lawson (gone)]

Attachment: removes duplicate variable declaration

Per Torben's comment in bug 295858. Woops.

Comment 6

[Stuart Morgan]

Comment on attachment 205143 [details] [diff] [review]
removes duplicate variable declaration

Looks good. r=me

Comment 7

[Simon Fraser [no longer active]]

Comment on attachment 205143 [details] [diff] [review]
removes duplicate variable declaration

I'm not sure I agree with this patch (or the way it was before any of these changes). The new setActive: line makes the content area active for the browser window that was frontmost, or opened. But we ignore the fact that loadInBackground may be YES, so the user may have focus in the URL bar, which we'll change.

I think we only want to bring the browser window to the front if we're loading in an empty tab, or !background.

Comment 8

[Stuart Morgan]

Good point.  In fact, that line will probably completely defeat the "load in background" pref--we've just never noticed because there aren't currently any code paths that call this with forceFront:NO and another window open.

Comment 9

[Simon Fraser [no longer active]]

Attachment: Patch

Comment 10

[Simon Fraser [no longer active]]

Fixed, b+t.

Comment 11

[Mike Pinkerton (not reading bugmail)]

this is still broken for me in 2005120804

if i have a window open and dblclick a webloc on the desktop, the new window starts in teh bg, comes to the fg, then may or may not jump back and forth a few times before it's done loading. sometimes it ends up in the bg, sometimes it ends up in the fg.

Comment 12

[Stuart Morgan]

Probably because Simon's patch didn't add the
controller = (BrowserWindowController*)[[self getFrontmostBrowserWindow] windowController];
in the openNewWindowWithURL case, so it's pulling the old window forward instead of the new window, like the earlier patch in the other bug.

Comment 13

[Simon Fraser [no longer active]]

Oh, now I get it, and the reason for that comment that we removed.

[BrowserWindowController openNewWindowWithURL:...] is evil because it returns a NEW window. Duh.

Comment 14

[Simon Fraser [no longer active]]

OK, I think I fixed it this time.

Comment 15

[Chris Lawson (gone)]

(In reply to comment #14)
> OK, I think I fixed it this time.

Possibly related issue:

http://forums.mozillazine.org/viewtopic.php?t=355209

I can't tell for sure if this is the same issue or not, but it sounds pretty similar. Might be worth a look, anyway.

cl