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

Status: VERIFIED FIXED

(Firefox :: Shell Integration, defect)

Description

[Tom Dyas]

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.)

Comment 1

[harsun]

(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.

Comment 2

[Tom Dyas]

Attachment: 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.

Comment 3

[Markus Stange [:mstange]]

Attachment: 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

[Alek Slater]

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).

Comment 5

[Josh Aas]

Taking, I've made some progress on this bug.

Comment 6

[Josh Aas]

Attachment: 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.

Comment 7

[Josh Aas]

Attachment: 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

Comment 8

[Josh Aas]

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.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

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);
});

Comment 10

[Josh Aas]

Attachment: fix v1.1

I like it, that is better. Thanks.

Comment 11

[Josh Aas]

pushed to mozilla-inbound

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

Comment 12

[Josh Aas]

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

Comment 13

[Josh Aas]

pushed to mozilla-aurora

http://hg.mozilla.org/releases/mozilla-aurora/rev/0a856c99ad3b

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/257580e4ccef

Comment 15

[Mihaela Velimiroviciu (:mihaelav)]

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.

Comment 16

[Federico Padua [:fedepad]]

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.

Comment 17

[u279076]

Thank you Federico.