The default bug view has changed. See this FAQ.

[Mac] Choosing "New Window" from Firefox's dock menu does not open the new window in the active Space

VERIFIED FIXED in Firefox 16

Status

()

Firefox
Shell Integration
VERIFIED FIXED
7 years ago
5 years ago

People

(Reporter: Tom Dyas, Assigned: Josh Aas)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 17
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox14 affected, firefox15 affected, firefox16 verified, firefox17 verified)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

7 years ago
In bug 415463 comment 115, a user reported that, when "New Window" from Firefox's dock menu is selected, Firefox does not open the new window in the active Space, but rather in the Space where the last active Firefox window is.  I have confirmed this behavior.

Safari's dock menu also includes a "New Window" item and will open the new window in the active Space.  Firefox should match Safari's behavior.

Here is a link to one person's pure Cocoa solution: http://osdir.com/ml/cocoa-dev/2009-11/msg01284.html  I am investigating whether a similar solution will work for Gecko.  (The fix may end up being in Widget:Cocoa.)
(Reporter)

Updated

7 years ago
Version: unspecified → Trunk

Comment 1

7 years ago
(In reply to comment #0)
> In bug 415463 comment 115, a user reported that, when "New Window" from
> Firefox's dock menu is selected, Firefox does not open the new window in the
> active Space, but rather in the Space where the last active Firefox window is. 
> I have confirmed this behavior.
> 
> Safari's dock menu also includes a "New Window" item and will open the new
> window in the active Space.  Firefox should match Safari's behavior.
> 
> Here is a link to one person's pure Cocoa solution:
> http://osdir.com/ml/cocoa-dev/2009-11/msg01284.html  I am investigating whether
> a similar solution will work for Gecko.  (The fix may end up being in
> Widget:Cocoa.)

I have a number of people here in my office, who want to know the fix of this one, the previous bug has a quick workaround, however I do think the person who raised "Bug 415463 - Dock menu support for Mac OS X (add "New Window" menu item)", was hoping for Bug 574229 fix as well.
thx.
(Reporter)

Comment 2

7 years ago
Created attachment 490784 [details] [diff] [review]
non-working quick fix

The attached patch is what I used to try and fix the bug.  I no longer have the time any more to work on this bug, including determining why OS X didn't respect the flag set in the patch.
(Reporter)

Updated

7 years ago
Assignee: tom.dyas → nobody
Created attachment 492373 [details] [diff] [review]
another non-working fix

I think the trick in the solution you referenced was to change the collection behavior around the call to makeKeyAndOrderFront. This patch does that, and it sometimes works, but most of the time the new window and the window on the other space immediately swap places...

Comment 4

5 years ago
This bug is still present, and not fixed after 2 years?
Is it not just using a key like NSWindowCollectionBehaviorMoveToActiveSpace, or something related to that (all in the NSWindow documentation).
(Assignee)

Comment 5

5 years ago
Taking, I've made some progress on this bug.
Assignee: nobody → joshmoz
(Assignee)

Comment 6

5 years ago
Created attachment 648928 [details] [diff] [review]
fix v0.8

It took me a surprisingly long time to come up with this simple patch. The discussion earlier in this bug threw me off the correct trail for a bit.

The problem isn't how we open a new window or what its collection behavior is. The problem is simply that we active the app from our dock code, which causes a spaces switch before the new window appears. Removing this activation is the basic fix, but we still need to figure out how to activate the window after it is opened. I should be able to figure that out in the next day or two.
Attachment #490784 - Attachment is obsolete: true
Attachment #492373 - Attachment is obsolete: true
(Assignee)

Comment 7

5 years ago
Created attachment 648939 [details] [diff] [review]
fix v1.0

I need to do some more testing and I'm not 100% sure this can't leak via the observer, but it seems to work pretty well in minimal testing.

OpenBrowserWindow() seems to be effectively asynchronous, so simply activating the application after calling OpenBrowserWindow() doesn't work. We still end up switching to the other space before the new window opens. My solution in this patch is to observe document-shown and activate when we receive the notification.

Try run here:

https://tbpl.mozilla.org/?tree=Try&rev=2e3d6d55b1d7
Attachment #648928 - Attachment is obsolete: true
(Assignee)

Comment 8

5 years ago
Comment on attachment 648939 [details] [diff] [review]
fix v1.0

Questions for reviewer: I'm not really sure that observing "document-shown" is the best idea, I just don't know of a better one. If that didn't fire for some reason we would misbehave and possibly leak something. Also, we might be able to execute that code sooner after the window has actually been opened.
Attachment #648939 - Flags: review?(gavin.sharp)
Comment on attachment 648939 [details] [diff] [review]
fix v1.0

Can you just use a load listener on the window instead? Something like:

let win = OpenBrowserWindow();
win.addEventListener("load", function listener() {
  win.removeEventListener("load", listener);
  dockSupport.activateApplication(true);
});
(Assignee)

Comment 10

5 years ago
Created attachment 649745 [details] [diff] [review]
fix v1.1

I like it, that is better. Thanks.
Attachment #648939 - Attachment is obsolete: true
Attachment #648939 - Flags: review?(gavin.sharp)
Attachment #649745 - Flags: review?(gavin.sharp)
Attachment #649745 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 11

5 years ago
pushed to mozilla-inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/257580e4ccef
(Assignee)

Comment 12

5 years ago
Comment on attachment 649745 [details] [diff] [review]
fix v1.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Dock integration
User impact if declined: This is a big deal for anyone who uses spaces on OS X and the patch is pretty safe. Hoping to uplift to Firefox 16.
Testing completed (on m-c, etc.): Landed on m-i today.
Risk to taking this patch (and alternatives if risky): Not very risky, easy to back out.
String or UUID changes made by this patch: None
Attachment #649745 - Flags: approval-mozilla-aurora?
Attachment #649745 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 13

5 years ago
pushed to mozilla-aurora

http://hg.mozilla.org/releases/mozilla-aurora/rev/0a856c99ad3b
status-firefox14: --- → affected
status-firefox15: --- → affected
status-firefox16: --- → fixed
status-firefox17: --- → fixed
https://hg.mozilla.org/mozilla-central/rev/257580e4ccef
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17

Updated

5 years ago
Blocks: 389614
Keywords: verifyme
I've verified this on:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:16.0) Gecko/20100101 Firefox/16.0 beta 4

A new window is opened in the active space.

Setting the flag to Verified on Firefox 16.
status-firefox16: fixed → verified
I tested on :

Firefox 17.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID : 20121005042010

The new window is opened in the active desktop, it works fine.
Thank you Federico.
Status: RESOLVED → VERIFIED
status-firefox17: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.