735946 - SM 2.8 Mac - SM Not Returned to Open from Minimize to Dock

Status: RESOLVED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[Rufus]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20120312 Firefox/11.0 SeaMonkey/2.8
Build ID: 20120312221231

Steps to reproduce:

I Minimized the open SM Browser window into the Dock by double clicking on the SM Taskbar, then with SM Minimized attempted to open either of an html file on my Desktop from the Finder by double-clicking and/or clicking a link from an e-mail.


Actual results:

SM does not come forward from the Dock; i.e. - SM remains minimized even though the file or link *does* open in a new SM tab per my Preference setting.  User has to click the Minimized SM icon in the Dock to restore SM to Open.  Same behavior occurs if user invokes command+M to Minimize.


Expected results:

If user opens a link or html file using Safari when minimized to the Dock, Safari returns to Open from the Dock and the new page or file is displayed automatically.  This is default OS X interface behavior, and I have also verified that this behavior is/was working properly in SM 2.7.2; broken in SM 2.8.

Comment 1

[Rufus]

Noticed in addition today that not only does the above occur, but that when SM is already in the Open state and a link is clicked in another Space, that does not bring the Space containing SM to front - i.e.; if I click a link in an e-mail in Space 1 and SM is open in Space 2, the OS does not switch to Space 2 even though the link is opened in a new tab in SM per my preference setting.

Again - this was working in SM 2.7.2; broken in SM 2.8.

Comment 2

[Stefan [:stefanh]]

Does this happen with Lion or snow Leaopard or does it happen with both OS versions?

Comment 3

[Stefan [:stefanh]]

I can reproduce this on 2.8 and trunk (I'm on Lion). I just checked 2.7.2, and it works there.

Comment 4

[Stefan [:stefanh]]

This is a bit tricky to test when you have multiple apps with the same version installed, but I found out that I could test by right-clicking on a file and then choose the "Other..." option (navigating to the app).

This regressed between 2011-11-30 and 2011-12-02 and backing out the patch in bug 703522 fixes the issue for me.

Comment 5

[[e-mail]]

The old tab switching code looked for something to focus, and because there wasn't anything yet, it went and focused the content area.
But the new code focuses the browser element instead, which works better when switching back to a document that uses frames, but also doesn't invoke the focus-raises-window effect.
Now, this doesn't normally matter, because most of our new tab callers focus the content area anyway, but obviously there exists one that does not...

Comment 6

[Rufus]

(In reply to Stefan [:stefanh] from comment #2)
> Does this happen with Lion or snow Leaopard or does it happen with both OS
> versions?

Both Sno Lep and Lion.

Comment 7

[Stefan [:stefanh]]

Attachment: Bring up the window if browser.tabs.loadDivertedInBackground is false

OK, this fixes the issue. If the pref is set to true, we don't bring up the window - that's probably correct (matches firefox).

Comment 8

[Philip Chee]

Comment on attachment 611279 [details] [diff] [review]
Bring up the window if browser.tabs.loadDivertedInBackground is false

>-        return gBrowser.getBrowserForTab(newTab).contentWindow;
>+        var contentWin = gBrowser.getBrowserForTab(newTab).contentWindow;
>+          if (!bgLoad)
>+            contentWin.focus();
Over indented?

Comment 9

[Stefan [:stefanh]]

(In reply to Philip Chee from comment #8)
> >+        var contentWin = gBrowser.getBrowserForTab(newTab).contentWindow;
> >+          if (!bgLoad)
> >+            contentWin.focus();
> Over indented?

Ah, yes. I'll fix that on check-in.

Comment 10

[H. Hofer]

I just tried the patch on Windows, and it seems to fix bug 736611 as well :-)

Comment 11

[Stefan [:stefanh]]

Attachment: Better indented

This doesn't seem to be mac-specific (comment #10), so maybe neil can have a look at the patch.

Comment 12

[H. Hofer]

This also affects 2.8 and beta...

Comment 13

[Philip Chee]

Comment on attachment 611497 [details] [diff] [review]
Better indented

> This also affects 2.8 and beta...
We are not touching 2.8, only Aurora and Beta.

[Approval Request Comment]
Regression caused by (bug #): bug 703522.
User impact if declined: additional user step needed to raise the SeaMonkey window when external links are opened in a minimized SeaMonkey.
Testing completed (on m-c, etc.): Tested on Lion and Windows7.
Risk to taking this patch (and alternatives if risky): Minimal, this is a regression fix.
String changes made by this patch: None.

Comment 14

[Jens Hatlak (:InvisibleSmiley)]

(In reply to H. Hofer from comment #10)
> I just tried the patch on Windows, and it seems to fix bug 736611 as well :-)

If that is the case, mark it as a duplicate of this one.

(In reply to Philip Chee from comment #13)
> > This also affects 2.8 and beta...
> We are not touching 2.8, only Aurora and Beta.

Then why only request Beta approval? Do you think there is a risk involved for Aurora, or anything else objecting?

Comment 15

[Philip Chee]

Comment on attachment 611497 [details] [diff] [review]
Better indented

Sorry, mis-fingered.

Comment 16

[Stefan [:stefanh]]

http://hg.mozilla.org/comm-central/rev/b3f800023045

Comment 17

[Stefan [:stefanh]]

I'll land on aurora/beta tomorrow or later on in the weekend.

Comment 19

[Trevor]

Target Milestone Seamonkey 2.11

Isn't an earlier integration possible for 4 lines of code?
Till 2.11 it will take about 4 months and that's the most annoying bug I've ever had since Netscape 1.

Comment 20

[Jens Hatlak (:InvisibleSmiley)]

(In reply to Trevor from comment #19)
> Target Milestone Seamonkey 2.11

Target Milestone equals the version of trunk at the time of the bug fix.

> Isn't an earlier integration possible for 4 lines of code?

The patch has already been approved for Aurora (to-be 2.10) and Beta (to-be 2.9), i.e. it'll be fixed for 2.9 for sure, probably our next beta 2.9b3 even.

Comment 21

[Trevor]

Good news :-)
Thanks for this information, Jens.

Comment 22

[Stefan [:stefanh]]

Landed on aurora and beta:
http://hg.mozilla.org/releases/comm-aurora/rev/b7d96060bc37
http://hg.mozilla.org/releases/comm-beta/rev/81b871cdd9f8