Closed Bug 319257 Opened 16 years ago Closed 16 years ago

URLs from external apps don't bring window to front

Categories

(Camino Graveyard :: General, defect)

PowerPC
macOS
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Camino1.0

People

(Reporter: bugzilla-graveyard, Assigned: sfraser_bugs)

References

Details

(Keywords: fixed1.8, regression)

Attachments

(2 files, 2 obsolete files)

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.
Attachment #205110 - Flags: review?
Status: NEW → ASSIGNED
Requesting blocking for 1.0.

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

cl
Flags: camino1.0?
Flags: camino1.0? → camino1.0+
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.
Attachment #205110 - Flags: review? → review-
Thanks for your patience, Stuart. How's that one?

cl
Attachment #205110 - Attachment is obsolete: true
Attachment #205119 - Flags: review?
Per Torben's comment in bug 295858. Woops.
Attachment #205119 - Attachment is obsolete: true
Attachment #205143 - Flags: review?
Attachment #205119 - Flags: review?
Comment on attachment 205143 [details] [diff] [review]
removes duplicate variable declaration

Looks good. r=me
Attachment #205143 - Flags: superreview?(sfraser_bugs)
Attachment #205143 - Flags: review?
Attachment #205143 - Flags: review+
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.
Attachment #205143 - Flags: superreview?(sfraser_bugs) → superreview-
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.
Attached patch PatchSplinter Review
Assignee: bugzilla → sfraser_bugs
Fixed, b+t.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
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.
Status: REOPENED → ASSIGNED
OK, I think I fixed it this time.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
(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
You need to log in before you can comment on or make changes to this bug.