Last Comment Bug 752149 - [OS X] nsCocoaWindow::ConstrainPosition uses wrong screen in multi-display setup
: [OS X] nsCocoaWindow::ConstrainPosition uses wrong screen in multi-display setup
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: 12 Branch
: x86 Mac OS X
: -- normal with 1 vote (vote)
: mozilla15
Assigned To: Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
:
: Markus Stange [:mstange]
Mentors:
: 750537 750999 754255 760148 762065 769688 770006 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-04 21:59 PDT by scottkaiser
Modified: 2012-07-30 14:33 PDT (History)
18 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
+
wontfix
+
fixed


Attachments
Patch v0.1 (1.92 KB, patch)
2012-05-16 18:25 PDT, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
smichaud: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta-
Details | Diff | Splinter Review

Description scottkaiser 2012-05-04 21:59:10 PDT
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
onepassword@agilebits.com
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
trackerblock@privacychoice.org
SearchLoad Options
0.6.3
true
searchloadoptions@esteban.torres
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] 2012-05-06 18:46:55 PDT
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 2012-05-06 20:16:02 PDT
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] 2012-05-08 13:19:11 PDT
Thanks Scott.

Also now sending over to Steven, which I failed to do previously.
Comment 4 Steven Michaud [:smichaud] (Retired) 2012-05-08 16:15:16 PDT
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) 2012-05-08 16:38:46 PDT
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 2012-05-08 18:37:32 PDT
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) 2012-05-08 18:53:20 PDT
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] 2012-05-09 14:04:25 PDT
bug 750999 is a duplicate of this one
Comment 9 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-15 13:14:25 PDT
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] 2012-05-15 13:50:02 PDT
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] 2012-05-15 14:42:14 PDT
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 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-15 15:04:24 PDT
Thanks Tracy, can you use the mozregression tool to track down a regression range? See comment 9.

Thanks
Comment 13 Tracy Walker [:tracy] 2012-05-15 15:12:13 PDT
I need no tool.

Regression occurred between 20111222 (works) and 20110123(bug) mozilla-central nightly builds
Comment 14 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-15 15:33:39 PDT
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 reading much bugmail, email directly) 2012-05-16 01:58:06 PDT
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) 2012-05-16 10:53:02 PDT
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] 2012-05-16 17:21:23 PDT
(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) 2012-05-16 17:36:16 PDT
On the face of it bug 356742 seems worse.  How many people use dual monitors?
Comment 19 Steven Michaud [:smichaud] (Retired) 2012-05-16 17:37:01 PDT
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) 2012-05-16 17:40:58 PDT
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 2012-05-16 18:04:50 PDT
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 reading much bugmail, email directly) 2012-05-16 18:12:09 PDT
(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 reading much bugmail, email directly) 2012-05-16 18:25:54 PDT
Created attachment 624618 [details] [diff] [review]
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) 2012-05-17 09:45:10 PDT
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) 2012-05-17 11:52:26 PDT
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 reading much bugmail, email directly) 2012-05-18 12:18:41 PDT
(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) 2012-05-18 12:43:00 PDT
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 reading much bugmail, email directly) 2012-05-18 15:19:14 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/09b5230bad3d
Comment 29 Matt Brubeck (:mbrubeck) 2012-05-19 06:37:56 PDT
https://hg.mozilla.org/mozilla-central/rev/09b5230bad3d
Comment 30 scottkaiser 2012-05-19 11:17:30 PDT
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) 2012-05-19 11:24:29 PDT
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) 2012-05-19 11:27:38 PDT
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) 2012-05-19 11:31:48 PDT
http://nightly.mozilla.org/ is a slightly easier way to install Firefox nightly.
Comment 34 Alex Keybl [:akeybl] 2012-05-21 15:49:26 PDT
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 2012-05-21 17:01:41 PDT
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 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-22 10:11:38 PDT
Thanks Scott, I believe that should satisfy the qawanted. Alex?
Comment 37 Alex Keybl [:akeybl] 2012-05-22 11:32:27 PDT
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] 2012-05-22 11:33:14 PDT
(that was assuming this fix is fairly low risk)
Comment 39 Steven Michaud [:smichaud] (Retired) 2012-05-22 11:40:01 PDT
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] 2012-05-22 11:43:18 PDT
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) 2012-05-22 11:45:29 PDT
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 43 Wayne Mery (:wsmwk, NI for questions) 2012-06-02 09:52:01 PDT
*** Bug 750537 has been marked as a duplicate of this bug. ***
Comment 44 Wayne Mery (:wsmwk, NI for questions) 2012-06-02 09:52:12 PDT
*** Bug 750999 has been marked as a duplicate of this bug. ***
Comment 45 Wayne Mery (:wsmwk, NI for questions) 2012-06-02 09:52:25 PDT
*** Bug 754255 has been marked as a duplicate of this bug. ***
Comment 46 Wayne Mery (:wsmwk, NI for questions) 2012-06-02 09:55:09 PDT
*** Bug 760148 has been marked as a duplicate of this bug. ***
Comment 47 Philip Chee 2012-06-10 11:46:08 PDT
*** Bug 762065 has been marked as a duplicate of this bug. ***
Comment 48 Philip Chee 2012-06-30 11:00:41 PDT
*** Bug 769688 has been marked as a duplicate of this bug. ***
Comment 49 Ludovic Hirlimann [:Usul] 2012-07-01 06:23:47 PDT
*** Bug 770006 has been marked as a duplicate of this bug. ***

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