752149 - [OS X] nsCocoaWindow::ConstrainPosition uses wrong screen in multi-display setup

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[scottkaiser]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:13.0) Gecko/20100101 Firefox/13.0
Build ID: 20120501201020

Steps to reproduce:

Upgraded to Firefox 12.0. I have a second monitor connected to my MacBook Pro. With Firefox 11 and all previous versions, I had it set up that Firefox would automatically open on the second monitor when I open the program. With Firefox 12, it absolutely will not do this any longer. Safari or Chrome correctly open on the 2nd monitor.

Here's what I tried:

-Resizing the window and moving it to the second display, then quitting the program and reopening it.

-Moving the window to the second display and maximizing it, then quitting the program and reopening it.

-Uninstalling Firefox and deleting ALL files (including those in the Library Application Support and Preferences folders), then reinstalling version 12.0

-Uninstalling Firefox and deleting ALL files (including those in the Library Application Support and Preferences folders), then reinstalling version 13.0b

-Uninstalling Firefox and deleting ALL files (including those in the Library Application Support and Preferences folders), then running disk utility to repair permissions and disk, THEN reinstalling version 12.0.

What will correct the problem (though not a solution):

-Uninstalling Firefox and deleting ALL files (including those in the Library Application Support and Preferences folders), then reverting back to reinstalling version 11 and turning off the automatic updates.

The tech specs:

MacBook Pro 15" 2.66 GHz Intel Core i7, 8 GB 1067 MHz DDR3

Mac OS X Lion 10.7.3 (11D50)

Intel HD Graphics:

 Chipset Model:	Intel HD Graphics
 Type:	GPU
 Bus:	Built-In
 VRAM (Total):	288 MB
 Vendor:	Intel (0x8086)
 Device ID:	0x0046
 Revision ID:	0x0018
 gMux Version:	1.9.21

NVIDIA GeForce GT 330M:

 Chipset Model:	NVIDIA GeForce GT 330M
 Type:	GPU
 Bus:	PCIe
 PCIe Lane Width:	x16
 VRAM (Total):	512 MB
 Vendor:	NVIDIA (0x10de)
 Device ID:	0x0a29
 Revision ID:	0x00a2
 ROM Revision:	3560
 gMux Version:	1.9.21
 Displays:

Color LCD:

 Resolution:	1440 x 900
 Pixel Depth:	32-Bit Color (ARGB8888)
 Main Display:	Yes
 Mirror:	Off
 Online:	Yes
 Built-In:	Yes

Sharp LL-191A-B:

 Resolution:	1280 x 1024 @ 75 Hz
 Pixel Depth:	32-Bit Color (ARGB8888)
 Display Serial Number:	4H065468    
 Mirror:	Off
 Online:	Yes
 Rotation:	Supported
 Adapter Type:	Apple Mini DisplayPort To VGA Adapter
 Adapter Firmware Version:	1.03


    Mac OS
    Firefox 12.0

This happened

Every time Firefox opened
This started when...

I upgraded to firefox version 12.
More Information

Application Basics
Name
Firefox
Version
12.0
User Agent
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0) Gecko/20100101 Firefox/12.0
Profile Directory
Show in Finder
Enabled Plugins
about:plugins
Build Configuration
about:buildconfig
Crash Reports
about:crashes
Memory Use
about:memory
Extensions
Name
Version
Enabled
ID
1Password
3.9.4
true
[e-mail]
Context Search
0.4.6
true
{902D2C4A-457A-4EF9-AD43-7014562929FF}
Password Exporter
1.2.1
true
{B17C1C5A-04B1-11DB-9804-B622A1EF5492}
PrivacyChoice TrackerBlock
2.2
true
[e-mail]
SearchLoad Options
0.6.3
true
[e-mail]
Important Modified Preferences
Name
Value
browser.cache.disk.capacity
1048576
browser.cache.disk.smart_size.first_run
false
browser.cache.disk.smart_size_cached_value
1048576
browser.places.smartBookmarksVersion
2
browser.startup.homepage_override.buildID
20120420145725
browser.startup.homepage_override.mstone
rv:12.0
browser.tabs.onTop
false
extensions.lastAppVersion
12.0
network.cookie.prefsMigrated
true
places.database.lastMaintenance
1335586198
places.history.expiration.transient_current_max_pages
104858
privacy.clearOnShutdown.cookies
false
privacy.clearOnShutdown.sessions
false
privacy.donottrackheader.enabled
true
privacy.popups.showBrowserMessage
false
privacy.sanitize.migrateFx3Prefs
true
privacy.sanitize.sanitizeOnShutdown
true
security.warn_viewing_mixed
false
Graphics
Vendor ID
0x10de
Device ID
0x a29
WebGL Renderer
NVIDIA Corporation -- NVIDIA GeForce GT 330M OpenGL Engine -- 2.1 NVIDIA-7.18.11
GPU Accelerated Windows
1/1 OpenGL
AzureBackend
quartz
Installed Plug-ins

    Shockwave Flash 11.2 r202
    Displays Java applet content, or a placeholder if Java is not installed.
    Adobe® Acrobat® Plug-in for Web Browsers, Version 10.1.3
    Microsoft Office for Mac SharePoint Browser Plug-in
    The QuickTime Plugin allows you to view a wide variety of multimedia content in web pages. For more information, visit the QuickTime Web site.
    5.0.61118.0
    The Flip4Mac WMV Plugin allows you to view Windows Media content using QuickTime. 

User Agent

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0) Gecko/20100101 Firefox/12.0


Actual results:

Firefox opened on the first monitor


Expected results:

Firefox should have remembered the window settings and opened on the second monitor.

Comment 1

[Alex Keybl [:akeybl]]

We won't re-spin FF12 for this, but we will track for the next couple of releases and try to get a fix in asap.

Steven - I'm starting this bug out with you. We've also heard of others running into this on SUMO. Apologies if this is a dupe.

Comment 2

[scottkaiser]

Thanks Alex.  I tried searching to make sure this had not been previously submitted before submitting it.  I have moved on to FF 13 beta where it is still a problem.  Please let me know if I can provide any additional information or do any testing for you.

Comment 3

[Alex Keybl [:akeybl]]

Thanks Scott.

Also now sending over to Steven, which I failed to do previously.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

I've got tons of more urgent stuff to attend to, and also don't have a second monitor.

It'll be some time before I can get to this.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Scott, please also try today's mozilla-central (aka trunk) nightly, to see if the problem still happens there.

ftp://ftp.mozilla.org/pub/firefox/nightly/2012-05-08-03-05-17-mozilla-central/firefox-15.0a1.en-US.mac.dmg

Comment 6

[scottkaiser]

Thank you Steven.  I really appreciate your help.  I am not the only one having this bug.  There is a thread on the Firefox Help Forums here that I originally posted before opening this bug: https://support.mozilla.org/questions/926289

I did download the nightly per your request, and I can verify that the bug is still existent in the 5/8 nightly.

Please let me know if there is anything I can do to assist.  Thanks!

Comment 7

[Steven Michaud [:smichaud] (Retired)]

Thanks, Scott.

It would help a lot if you could find the last mozilla-central nightly unaffected by this problem, and the first mozilla-central nightly that has it.

For this information to be useful to us, the gap between these two nightlies (i.e. the regression range) can't be larger than one day.

With this information I might be able to figure out the cause of this bug, even if I can't reproduce it or fix it.

Comment 8

[[:philipp]]

bug 750999 is a duplicate of this one

Comment 9

[u279076]

RE: qawanted
I've circulated this around QA; someone should be able to test this. In the meantime, Scott, can you work this down to a particular nightly where this started happening? Here are the Firefox 12 nightlies:

First: ftp://ftp.mozilla.org/pub/firefox/nightly/2011/12/2011-12-21-03-12-26-mozilla-central/
Last: ftp://ftp.mozilla.org/pub/firefox/nightly/2012/01/2012-01-31-03-11-50-mozilla-central/

Alternatively you can use http://harthur.github.com/mozregression/ to simplify the process.

Comment 10

[Tracy Walker [:tracy]]

I am able to reproduce this.  I my case, Fx12 initially opens on the connected monitor.  Which is where I want it.  However, if I move Fx to the laptop screen, then restart it.  It opens wedged against the bottom of the attached monitor, not where it was closed from

This is a Session Restore bug.  If you select to restore the session from the home page, the browser then jumps to the location it was at in previous session shut down.

Fx11 opens where in the location it was closed from.  

I'll work on finding a regression range, unless the new information is enough to nail down what is going on.

Comment 11

[Tracy Walker [:tracy]]

If I set my Preferences > General > Startup > When Firefox starts: > "Show my windows and tabs from last time," Firefox behaves correctly on restart by launching in previous session location.

Comment 12

[u279076]

Thanks Tracy, can you use the mozregression tool to track down a regression range? See comment 9.

Thanks

Comment 13

[Tracy Walker [:tracy]]

I need no tool.

Regression occurred between 20111222 (works) and 20110123(bug) mozilla-central nightly builds

Comment 14

[u279076]

Thanks Tracy. Looking at pushlog, the only thing I'm seeing is a PGO merge:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2011-12-22&enddate=2011-12-23

The only thing which stands out there is:
Bug 356742 -Implement nsCocoaWindow::ConstrainPosition()
http://hg.mozilla.org/mozilla-central/rev/163c2800f87f

Comment 15

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Looks like this may be a widget after all then. Let's avoid bouncing it around again though until we can be sure.

Steven, I know you're busy but you have blame on the suspected bug - is this something you could look into? I'll have access to a 2nd monitor to test tomorrow so if I get the time I'll try to see what I can figure out too.

Comment 16

[Steven Michaud [:smichaud] (Retired)]

I don't have access to a second monitor.

I'll probably buy a new one next week (I've been meaning to do that for a while, for other reasons).  But even then I may be too busy to get to this for a while -- my life lately has been like hospital triage in a war zone.

Thanks, nonetheless, for pointing out that one of "my" bugs may have been responsible for this.  That'll probably give me a leg up when I finally get to this.

Comment 17

[Alex Keybl [:akeybl]]

(In reply to Steven Michaud from comment #16)
> I'll probably buy a new one next week (I've been meaning to do that for a
> while, for other reasons).  But even then I may be too busy to get to this
> for a while -- my life lately has been like hospital triage in a war zone.

Is there any reason to leave bug 356742 in FF13, or can we back it out? This regression seems much worse than what's described in bug 356742.

Comment 18

[Steven Michaud [:smichaud] (Retired)]

On the face of it bug 356742 seems worse.  How many people use dual monitors?

Comment 19

[Steven Michaud [:smichaud] (Retired)]

We also of course don't yet know for sure that the patch for bug 356742 actually regressed this bug.

Comment 20

[Steven Michaud [:smichaud] (Retired)]

Also, bug 356742 involves a window (the sheet) completely disappearing.

This bug (as best I can tell) doesn't.  And it can easily be corrected by moving the window back to the second monitor.

Yes, this bug is annoying.  But it doesn't sound nearly as bad as bug 356742.

If I've understood this bug correctly.

Comment 21

[scottkaiser]

I finally found some time to do the testing on the nightlies you asked for earlier in the thread, but if I understand the subsequent comments correctly, it looks like you have isolated where and when the change that caused this occurred.

If you still need me to do testing or if I can be of assistance, please let me know.  My technical knowledge is no where near any of yours though, and I have absolutely no experience with programming or coding (though I'd like to learn someday).

Thank you again for looking into this.  All the hard work you do is very much appreciated.  I've been a loyal Firefox user since the very first beta.

Comment 22

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to Tracy Walker [:tracy] from comment #10)
> This is a Session Restore bug.  If you select to restore the session from
> the home page, the browser then jumps to the location it was at in previous
> session shut down.

This actually makes it expressly not a session restore bug. Session restore puts it at the right location but the non-restore behavior is what's broken - specifically loading position from localstore.rdf.

(In reply to Steven Michaud from comment #19)
> We also of course don't yet know for sure that the patch for bug 356742
> actually regressed this bug.

At this point, I'm almost certain it does, or at least can fix the bug here in ConstrainPosition.

(In reply to Steven Michaud from comment #20)
> This bug (as best I can tell) doesn't.  And it can easily be corrected by
> moving the window back to the second monitor.
> 
> Yes, this bug is annoying.  But it doesn't sound nearly as bad as bug 356742.

I think I agree, annoying but definitely not cause to panic. And again, to make the affected group size smaller, this is OS X users with a 2nd monitor, who have put the Firefox window on the 2ndary monitor, and don't have session restore on.


Anyway, if I had to guess, I would say that [mWindow screen] isn't giving the right screen. The Windows code uses nsIScreenManager.screenForRect to get an nsIScreen and then just gets a rect from that. Doing that here too fixes this bug (I haven't checked what it does in regards to bug 356742)

I can turn that into a real patch but I don't know if we'd want to land that on beta at this point or just back out what's there now or just live with it for another release.

Comment 23

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

This is pretty much straight from the Windows code and I haven't used enough XPCOM from native code to know if this cleans up right (I think it does with the comptrs) but it works.

Comment 24

[Steven Michaud [:smichaud] (Retired)]

This looks reasonable to me.  But I won't r+ it until I've tested it in a Seamonkey  build.

Note that neither of us knows why this works.  So (presuming it doesn't regress bug 356742) it'll need to bake for a while before we know that it doesn't have some *other* bad side effect.

Under the circumstances I don't think we should try to get this into FF 13.  And we definitely shouldn't back out the patch for bug 356742 -- that really is a much worse bug than this one.

When I wrote the patch for bug 356742 I also looked at the other widgets' implementations of nsIWidget::ConstrainPosition().  Windows uses XPCOM code to get the screen dimensions, but GTK2 uses "native" code (gdk_screen_width() and gdk_screen_height()).  I chose to use "native" methods in my patch (those called from nsCocoaUtils::CocoaRectToGeckoRect()), thinking they were more likely to be accurate.  I guess they weren't ... but we still don't know why.

Comment 25

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 624618 [details] [diff] [review]
Patch v0.1

> But I won't r+ it until I've tested it in a Seamonkey  build.

I ended up testing a Thunderbird build.

It doesn't regress bug 356742 (using the STR from bug 356742 comment #11).

Comment 26

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to Steven Michaud from comment #24)
> When I wrote the patch for bug 356742 I also looked at the other widgets'
> implementations of nsIWidget::ConstrainPosition().  Windows uses XPCOM code
> to get the screen dimensions, but GTK2 uses "native" code
> (gdk_screen_width() and gdk_screen_height()).  I chose to use "native"
> methods in my patch (those called from
> nsCocoaUtils::CocoaRectToGeckoRect()), thinking they were more likely to be
> accurate.  I guess they weren't ... but we still don't know why.

I'm curious now if the GTK code is broken too or maybe it gdk_screen_width/height give back dimensions for a stitched screen.

I think the reason we're broken here is that the window hasn't been moved yet. From nsXULWindow::LoadPositionFromXUL we build up a position, then constrain it, then move the window. So assuming that we open on the main screen by default, by using [mWindow screen], it's going to be the wrong screen for this particular case (I did just confirm that screenBounds had the size info for the primary screen).

Comment 27

[Steven Michaud [:smichaud] (Retired)]

Sounds reasonable ... but of course I won't be able to test myself until I get a second monitor :-)

Comment 28

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/09b5230bad3d

Comment 29

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/09b5230bad3d

Comment 30

[scottkaiser]

I see Matt's comment above that this has been resolved/fixed.  Although I originally reported the bug, I don't know your processes. When/where is the fix added to a nightlie that I can test?  What version of Firefox is the fix scheduled to be added to - FF14?  Thanks!

Comment 31

[Steven Michaud [:smichaud] (Retired)]

We mark bugs "fixed" as soon as a patch gets into any of our regular channels -- in this case the "trunk" (aka mozilla-central).

If this bug's fix continues to follow the regular channels, the first release it'll get into is FF 15 (note that the "target milestone" has been set to "mozilla15").  But along the way we may decide to accelerate its progress, and get it into FF 14, or even possibly FF 13 (though I've argued above against getting it into FF 13, which is due out soon).

Comment 32

[Steven Michaud [:smichaud] (Retired)]

Scott, please try tomorrow's mozilla-central nightly, which will contain the patch for this bug:

ftp://ftp.mozilla.org/pub/firefox/nightly/

(You won't be able to download it until tomorrow).

Comment 33

[Matt Brubeck (:mbrubeck)]

http://nightly.mozilla.org/ is a slightly easier way to install Firefox nightly.

Comment 34

[Alex Keybl [:akeybl]]

If this is low risk enough, please nominate for FF13 approval to make it in before tomorrow's go-to-build. Adding qawanted to test the latest nightly with multiple displays.

Comment 35

[scottkaiser]

I just downloaded and tested the latest nightly.  I originally opened this bug report, and I can confirm the problem was fixed in the nightly I tested.  

Thank you everyone for your hard work and attention to this problem.

Comment 36

[u279076]

Thanks Scott, I believe that should satisfy the qawanted. Alex?

Comment 37

[Alex Keybl [:akeybl]]

Comment on attachment 624618 [details] [diff] [review]
Patch v0.1

[Triage Comment]
Thanks for testing - preapproving for Aurora 14 and Beta 13. Please land asap to make it into our fifth beta.

Comment 38

[Alex Keybl [:akeybl]]

(that was assuming this fix is fairly low risk)

Comment 39

[Steven Michaud [:smichaud] (Retired)]

I still think we shouldn't land this in FF 13.

Though Paul has given a plausible explanation of why/how his patch works in comment #26, neither of us has actually tested it.  Also this bug, though annoying, has an easy workaround.

Under the circumstances I think we should wait for FF 14.

Comment 40

[Alex Keybl [:akeybl]]

Comment on attachment 624618 [details] [diff] [review]
Patch v0.1

Given Steven's concerns about this causing worse regressions for FF13, we won't go against his recommendation to let this ride FF14 and up. Thanks Steven.

Comment 41

[Steven Michaud [:smichaud] (Retired)]

By testing the explanation I mean adding a bunch of NSLog/printf statements, to see if what they log confirms or denies the explanation.

> Thanks Steven.

No problem.  I just think it's better to be cautious :-)

Comment 42

[Marco Bonardo [:mak]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/10b7f6c9be4f