[windows] Using dual monitors, Firefox/Thunderbird opens in wrong monitor

RESOLVED FIXED in mozilla32

Status

()

defect
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: sawrubh, Assigned: emk)

Tracking

unspecified
mozilla32
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 4 obsolete attachments)

This is the bug for tracking the Linux specific fix for bug 264030.

Some additional comments from https://bugzilla.mozilla.org/show_bug.cgi?id=264030#c51 :

I have the opposite problem: Firefox always opens on my right-most (primary) monitor, even though I always close it (maximized) on my secondary (left-most) monitor.

Multi-monitor application bugs are one of my pet peeves, partially because Windows has two functions (GetWindowPlacement/SetWindowPlacement) that will do it correctly for you (and handles all the corner cases).  Don't try to reinvent the wheel, you'll most likely screw it up.  If you're not using GetWindowPlacement/SetWindowPlacement (which apparently Firefox isn't), then you've probably (in this case, definitely) overlooked something.
(In reply to Saurabh Anand [:sawrubh] from comment #0)
> This is the bug for tracking the Linux specific fix for bug 264030.
> 
Sorry I meant 'bug for tracking the Windows specific fix'.
> Some additional comments from
> https://bugzilla.mozilla.org/show_bug.cgi?id=264030#c51 :
> 
> I have the opposite problem: Firefox always opens on my right-most (primary)
> monitor, even though I always close it (maximized) on my secondary
> (left-most) monitor.
> 
> Multi-monitor application bugs are one of my pet peeves, partially because
> Windows has two functions (GetWindowPlacement/SetWindowPlacement) that will
> do it correctly for you (and handles all the corner cases).  Don't try to
> reinvent the wheel, you'll most likely screw it up.  If you're not using
> GetWindowPlacement/SetWindowPlacement (which apparently Firefox isn't), then
> you've probably (in this case, definitely) overlooked something.
Summary: Windows tracking bug for 264030 → Windows tracking bug for 264030 - Using dual monitors, Firefox/Thunderbird opens in wrong monitor
Summary: Windows tracking bug for 264030 - Using dual monitors, Firefox/Thunderbird opens in wrong monitor → [windows] Using dual monitors, Firefox/Thunderbird opens in wrong monitor
Note: For anyone working on this bug, the Mac OS X patch in bug 1005620 might be a useful starting point or reference.
https://tbpl.mozilla.org/?tree=Try&rev=70fd1771f746
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #8420493 - Flags: review?(jmathies)
Attachment #8420495 - Flags: review?(jmathies)
For anyone interested in this bug: a test build is available here.
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/VYV03354@nifty.ne.jp-fdb7378d1518/try-win32/
Comment on attachment 8420493 [details] [diff] [review]
Part 1: Add nsIWidget::GetRestoredBounds()

>+NS_METHOD nsBaseWidget::GetRestoredBounds(nsIntRect &aRect)
>+{
>+  return NS_ERROR_NOT_IMPLEMENTED;
I'd prefer something like
if (SizeMode() != nsSizeMode_Normal)
  return NS_ERROR_FAILURE;
return GetBounds(aRect);

This would then simplify the calling code.
I couldn't shortcut to GetBounds() because subclasses may override GetScreenBounds():
https://mxr.mozilla.org/mozilla-central/source/widget/gtk/nsWindow.cpp?rev=6c1c7e45c902#1423
Attachment #8420493 - Attachment is obsolete: true
Attachment #8420493 - Flags: review?(jmathies)
Attachment #8420662 - Flags: review?(roc)
Attachment #8420494 - Attachment is obsolete: true
Attachment #8420494 - Flags: review?(neil)
Attachment #8420663 - Flags: review?(neil)
Attachment #8420495 - Attachment is obsolete: true
Attachment #8420495 - Flags: review?(jmathies)
Attachment #8420664 - Flags: review?(jmathies)
Comment on attachment 8420662 [details] [diff] [review]
Part 1: Add nsIWidget::GetRestoredBounds()

Review of attachment 8420662 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/nsIWidget.h
@@ +1088,5 @@
>      NS_IMETHOD GetScreenBounds(nsIntRect &aRect) = 0;
>  
>      /**
> +     * Similar to GetScreenBounds except that this function will always.
> +     * get the normal (restored) widget's dimensions.

This comment isn't clear enough. What exactly does "the normal (restored) widget's dimensions" mean?
Attachment #8420662 - Flags: review?(roc) → review-
Clarified the comment.
Attachment #8420662 - Attachment is obsolete: true
Attachment #8420765 - Flags: review?(roc)
Comment on attachment 8420765 [details] [diff] [review]
Add nsIWidget::GetRestoredBounds()

Review of attachment 8420765 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent, thanks!
Attachment #8420765 - Flags: review?(roc) → review+
Attachment #8420664 - Flags: review?(jmathies) → review+
Comment on attachment 8420663 [details] [diff] [review]
Part 2: Use nsIWidget::GetRestoredBounds from appshell

Review of attachment 8420663 [details] [diff] [review]:
-----------------------------------------------------------------

Wrong person. Please make sure you assign to the correct Neil, guys.
Attachment #8420663 - Flags: review?(neil) → review-
Comment on attachment 8420663 [details] [diff] [review]
Part 2: Use nsIWidget::GetRestoredBounds from appshell

Oops.
Attachment #8420663 - Flags: review- → review?(neil)
Comment on attachment 8420663 [details] [diff] [review]
Part 2: Use nsIWidget::GetRestoredBounds from appshell

Thanks, that's looking a lot better. I've only had time for a quick look so far, so I'll just point out that sizeMode could be moved down into the PAD_MISC block and gotRestoredBounds looks a little lonely on its own line ;-) Don't bother updating the patch just yet in case I find something else.
Comment on attachment 8420663 [details] [diff] [review]
Part 2: Use nsIWidget::GetRestoredBounds from appshell

I have a problem. I can't actually reproduce the bug as described. I can for example configure Thunderbird to open the mail window maximised on the primary monitor and the address book maximised on the secondary monitor and it remembers which window should open on which monitor. Could someone provide me some STR?
The key to reproduce is that the monitor it appears on when starting is the one on which it was last in a non-maximized state on.

The common way to do this is to use the Aero Snap operations on Windows to move a maximized window between monitors (Win+Shift+Left/Right) and then close it.
I dunno about Thunderbird.
But i know it on Firefox:
- Move Firefox to second screen
- Maximise window
- Close Firefox
- Open Firefox again
Now you will find it on primary screen again.
(In reply to neil@parkwaycc.co.uk from comment #18)
> Comment on attachment 8420663 [details] [diff] [review]
> I have a problem. I can't actually reproduce the bug as described. I can for
> example configure Thunderbird to open the mail window maximised on the
> primary monitor and the address book maximised on the secondary monitor and
> it remembers which window should open on which monitor. Could someone
> provide me some STR?

Hm, certainly the comment #20 issue is not reproducible for me, either. But it looks like this patch still fix the Aero Snap issue (comment #19).
(In reply to Masatoshi Kimura from comment #21)
> (In reply to comment #18)
> > Comment on attachment 8420663 [details] [diff] [review]
> > I have a problem. I can't actually reproduce the bug as described. I can for
> > example configure Thunderbird to open the mail window maximised on the
> > primary monitor and the address book maximised on the secondary monitor and
> > it remembers which window should open on which monitor. Could someone
> > provide me some STR?
> 
> Hm, certainly the comment #20 issue is not reproducible for me, either. But
> it looks like this patch still fix the Aero Snap issue (comment #19).

http://stackoverflow.com/questions/5673242/getwindowplacement-gives-the-incorrect-window-position suggests that using Aero Snap to switch monitors doesn't update the restored window placement.
(In reply to neil@parkwaycc.co.uk from comment #22)
> http://stackoverflow.com/questions/5673242/getwindowplacement-gives-the-
> incorrect-window-position suggests that using Aero Snap to switch monitors
> doesn't update the restored window placement.

The questioner must have misunderstood something. Apps should remember the un-Aero-Snapped dimensions and GetWindowPlacement returns the dimensions correctly. GetWindowsRect returns the Aero-Snapped dimensions. Apps bundled with Windows (such as IE or Notepad) will also remember the un-Aero-Snapped dimensions.
(In reply from comment #22)
> (In reply to Masatoshi Kimura from comment #21)
> > (In reply to comment #18)
> > > I have a problem. I can't actually reproduce the bug as described. I can for
> > > example configure Thunderbird to open the mail window maximised on the
> > > primary monitor and the address book maximised on the secondary monitor and
> > > it remembers which window should open on which monitor. Could someone
> > > provide me some STR?
> > 
> > Hm, certainly the comment #20 issue is not reproducible for me, either. But
> > it looks like this patch still fix the Aero Snap issue (comment #19).
> 
> http://stackoverflow.com/questions/5673242/getwindowplacement-gives-the-
> incorrect-window-position suggests that using Aero Snap to switch monitors
> doesn't update the restored window placement.

Ah, maybe I misunderstood that question. Anyway, I was able to borrow a dual-monitor Windows 7 PC and verified that the Aero Snap bug exists, but I don't have the authority to install an unreleased Firefox on that machine so I'll have to assume the code works by inspection.
Comment on attachment 8420663 [details] [diff] [review]
Part 2: Use nsIWidget::GetRestoredBounds from appshell

(Don't forget to consider my minor nits in comment #17.)
Attachment #8420663 - Flags: review?(neil) → review+
What's the behavior with 2 more monitors (say 3 e.g.)? Does it work as it should be?
Should work with 3 or more monitors.
I landed a trivial fixup for GCC (mingw) builds:

https://hg.mozilla.org/integration/mozilla-inbound/rev/781f4aff3769
Is it possible this caused bug 1250319?
You need to log in before you can comment on or make changes to this bug.