402176 - sanitize/optimize Cocoa nsIWidget move and resize implementations

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Josh Aas]

Cocoa's nsIWidget move and resize methods are not well implemented. We should fix all of it with the following goals in mind:

1. Don't do drawing we don't need to. No need to redraw the parent widget.
2. Don't call setFrame more often than we need to.
3. Clean up the code in general. It is just badly written and hard to understand now.
4. More bulletproofing (null checks) would be nice. Obviously we can't move or resize a widget that doesn't exist.

Comment 1

[Josh Aas]

Attachment: fix v1.0

Comment 2

[Josh Aas]

Note that this is not an attempt to "fix" 355177 - this is simply a step in the right direction should we choose to follow up on that bug.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 287071 [details] [diff] [review]
fix v1.0


>Index: widget/src/cocoa/nsCocoaWindow.mm
>===================================================================

> NS_IMETHODIMP nsCocoaWindow::Move(PRInt32 aX, PRInt32 aY)

>+  // The point we have is in Gecko coordinates (origin top-left). Convert
>+  // it to Cocoa ones (origin bottom-left).
>+  NSPoint coord = {aX, FlippedScreenY(aY)};
>+  [mWindow setFrameTopLeftPoint:coord];
>+

Hm, is this right?  I realize this is what the old code did, but it seems weird to me -- if we flip the Y coord, why do we call setFrameTopLeftPoint?

Comment 4

[Josh Aas]

I found that odd too and tried to not do that, but if you don't flip the y coord it breaks a lot of things. Windows go to the wrong side of the screen. I had planned to dig into that more after this.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 287071 [details] [diff] [review]
fix v1.0

Ugh, well, the stuff fixed by this patch is good in any case, but we should figure out the coordinate system stuff.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 287071 [details] [diff] [review]
fix v1.0

@@ -1,8 +1,9 @@
+
 /* -*- Mode: objc; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */

Eliminate

Comment 7

[Josh Aas]

This is important enough to block I think.

Comment 8

[Josh Aas]

landed on trunk