Window position is not persisted (persist="screenX screenY" doesn't work)

RESOLVED FIXED in mozilla1.9alpha5

Status

()

Core
Widget: Cocoa
--
major
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: mossop, Assigned: cbarrett)

Tracking

({regression})

Trunk
mozilla1.9alpha5
x86
Mac OS X
regression
Points:
---
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

1.31 KB, patch
Josh Aas
: review+
Mike Pinkerton (not reading bugmail)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
Window position is not being positioned correctly. This is reproducable almost all the time for me. Just close the window, open a new one and it will display in a new position, particularly when the window does not already fill the screen.

Just testing now, I close the window in about the middle of the screen and when I reopen it's the same size but at the top left. Quite often though when the window fills the screen, opening it again will position it about 100-200 pixels lower so some of the window is off the bottom of the screen.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a4pre) Gecko/20070407 Minefield/3.0a4pre ID:2007040704 [cairo]
Severity: normal → major
Flags: blocking1.9?
Summary: Window position is not persisted → Window position is not persisted (persist="screenX screenY" doesn't work)

Updated

11 years ago
Flags: blocking1.9? → blocking1.9+
(Assignee)

Updated

11 years ago
Assignee: joshmoz → cbarrett

Updated

11 years ago
Target Milestone: --- → mozilla1.9alpha5
(Assignee)

Comment 1

11 years ago
Created attachment 264181 [details] [diff] [review]
fix v1.0

The comment above NSRect frameRect =... should probably be changed, wasn't sure entirely what it should say though, since I'm not clear on why that works :-)

Other than that, this seems to fix the problem totally for me.
Attachment #264181 - Flags: review?(joshmoz)
(Assignee)

Updated

11 years ago
Attachment #264181 - Flags: review?(mano)
(Assignee)

Comment 2

11 years ago
Created attachment 264197 [details] [diff] [review]
fix v1.0.1

Forgot to use -u8p on the last patch, this one is properly formatted.
Attachment #264181 - Attachment is obsolete: true
Attachment #264197 - Flags: review?(mano)
Attachment #264181 - Flags: review?(mano)
Attachment #264181 - Flags: review?(joshmoz)
(Assignee)

Updated

11 years ago
Attachment #264197 - Flags: review?(joshmoz)

Comment 3

11 years ago
Comment on attachment 264197 [details] [diff] [review]
fix v1.0.1

nsCocoaWindow.mm:1187: warning: converting to 'nscoord' from 'float'

That needs to be fixed, it happens twice.

+  // Use the content view x & y, seems to work

Just get rid of that comment, everything it says is obvious from the code under it.

Otherwise this looks fine, please post a new patch and I'll r+ it with those fixes.
Attachment #264197 - Flags: review?(mano)
Attachment #264197 - Flags: review?(joshmoz)
Attachment #264197 - Flags: review-
(Assignee)

Comment 4

11 years ago
Created attachment 264248 [details] [diff] [review]
fix v1.1

Removed the comment and got rid of the nscoord warning.
Attachment #264197 - Attachment is obsolete: true
Attachment #264248 - Flags: review?(joshmoz)

Updated

11 years ago
Attachment #264248 - Flags: superreview?(mikepinkerton)
Attachment #264248 - Flags: review?(joshmoz)
Attachment #264248 - Flags: review+
Comment on attachment 264248 [details] [diff] [review]
fix v1.1

sr=pink

i'm assuming gecko expects coords in the cocoa coordinate system now? we don't have to convert them to 0,0 = topLeft?
Attachment #264248 - Flags: superreview?(mikepinkerton) → superreview+
(Assignee)

Comment 6

11 years ago
It looks like it doesn't matter what we set refPoint to, as long as it's set. Setting it to 0,0 results in the correct behavior, which makes some sense because the XUL window code is fetching the window position (nsXULWindow.cpp:1429).

What mystifies me is why if we don't have a refpoint set, it behaves erratically. Mano, you initially suggested setting the refPoint, any ideas?
(Assignee)

Comment 7

11 years ago
Created attachment 264293 [details] [diff] [review]
fix v1.2

After talking with Mano about it, I'm using the point that GetScreenBounds returns, since that's where screenX and screenY get their data, and that's the coordinate system everything else is in, apparently.
Attachment #264248 - Attachment is obsolete: true
Attachment #264293 - Flags: superreview?(mikepinkerton)
Attachment #264293 - Flags: review?(joshmoz)

Updated

11 years ago
Attachment #264293 - Flags: review?(joshmoz) → review+
Comment on attachment 264293 [details] [diff] [review]
fix v1.2

sr=pink
Attachment #264293 - Flags: superreview?(mikepinkerton) → superreview+

Comment 9

11 years ago
landed on trunk
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.