Last Comment Bug 735946 - SM 2.8 Mac - SM Not Returned to Open from Minimize to Dock
: SM 2.8 Mac - SM Not Returned to Open from Minimize to Dock
Status: RESOLVED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: seamonkey2.11
Assigned To: Stefan [:stefanh]
:
:
Mentors:
: 736611 (view as bug list)
Depends on:
Blocks: 703522 736611
  Show dependency treegraph
 
Reported: 2012-03-14 17:58 PDT by Rufus
Modified: 2012-04-05 07:20 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
wontfix
fixed
fixed


Attachments
Bring up the window if browser.tabs.loadDivertedInBackground is false (1.21 KB, patch)
2012-04-01 08:12 PDT, Stefan [:stefanh]
no flags Details | Diff | Splinter Review
Better indented (1.21 KB, patch)
2012-04-02 10:07 PDT, Stefan [:stefanh]
neil: review+
bugspam.Callek: approval‑comm‑aurora+
bugspam.Callek: approval‑comm‑beta+
Details | Diff | Splinter Review

Description Rufus 2012-03-14 17:58:46 PDT
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 2012-03-16 13:58:25 PDT
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] 2012-03-17 07:58:17 PDT
Does this happen with Lion or snow Leaopard or does it happen with both OS versions?
Comment 3 Stefan [:stefanh] 2012-03-17 08:13:11 PDT
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] 2012-03-17 09:12:15 PDT
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 neil@parkwaycc.co.uk 2012-03-17 09:37:06 PDT
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 2012-03-17 12:14:16 PDT
(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] 2012-04-01 08:12:11 PDT
Created attachment 611279 [details] [diff] [review]
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 2012-04-01 12:05:05 PDT
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] 2012-04-01 14:21:19 PDT
(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 2012-04-02 05:24:07 PDT
I just tried the patch on Windows, and it seems to fix bug 736611 as well :-)
Comment 11 Stefan [:stefanh] 2012-04-02 10:07:34 PDT
Created attachment 611497 [details] [diff] [review]
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 2012-04-02 13:10:46 PDT
This also affects 2.8 and beta...
Comment 13 Philip Chee 2012-04-03 02:08:48 PDT
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) 2012-04-03 02:24:52 PDT
(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 2012-04-03 05:42:58 PDT
Comment on attachment 611497 [details] [diff] [review]
Better indented

Sorry, mis-fingered.
Comment 16 Stefan [:stefanh] 2012-04-03 09:20:47 PDT
http://hg.mozilla.org/comm-central/rev/b3f800023045
Comment 17 Stefan [:stefanh] 2012-04-04 09:20:50 PDT
I'll land on aurora/beta tomorrow or later on in the weekend.
Comment 18 Philip Chee 2012-04-04 12:34:02 PDT
*** Bug 736611 has been marked as a duplicate of this bug. ***
Comment 19 Trevor 2012-04-04 13:49:54 PDT
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) 2012-04-04 13:55:53 PDT
(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 2012-04-04 14:08:59 PDT
Good news :-)
Thanks for this information, Jens.

Note You need to log in before you can comment on or make changes to this bug.