254191 - Cleanup Win32 nsWindow.cpp / nsWindow.h

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Dainis Jonitis]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040707 Firefox/0.9.2
Build Identifier: 

This is a follow-up to Bug 252067. This is the cleanup part.

1. Fix spelling in comments for nsIWidget.h, windows/nsWindow.cpp
   and windows/nsWindow.cpp
2. Replace all tabulation symbols with spaces. Make all source code to use
   the consistent indentation (2 spaces).
3. Where possible replace different implementations that look for topmost HWND
   with new function GetTopLevelHWND which was added in 252067.
4. Replace GetNativeData(NS_NATIVE_WINDOW) with mWnd, when call applies
   to nsWindow class or 'this', rather than nsIWidget interface pointer.
5. When handling WM_WINDOWPOSCHANGED make a call to ::RedrawWindow API function
   only when window becomes larger, thus avoiding useless system calls.
6. Do not use empty 'for' loops for delays in paint flashing
7. Some other minor cleanup


Reproducible: Always
Steps to Reproduce:

Comment 1

[Dainis Jonitis]

Attachment: diff -d -u -8

Comment 2

[Dainis Jonitis]

Attachment: diff -d -u -8 -w (for review purposes only)

Comment 3

[Ere Maijala (slow)]

Comment on attachment 155105 [details] [diff] [review]
diff -d -u -8

+    // its one of our windows so go ahead and send a message to it

its -> it's

+NS_METHOD nsWindow::Resize(PRInt32 aX, PRInt32 aY, PRInt32 aWidth, PRInt32
aHeight, PRBool  aRepaint)

Extra space before aRepaint.

-  //}

This in SetCursor belongs to the commented-out if in the beginning. I'd leave
it in. 

-	 }
-	 break;
+    } break;

I don't think this is a good change. The current style is predominant and good.

-	       if (newWidth != mLastSize.width)
+	 if (newWidth > mLastSize.width)

Are you sure this doesn't break anything if a window gets narrower? 

-	 for (x = 0; x < 1000000; x++);
+      PR_Sleep(PR_MillisecondsToInterval(30));

A good guess or a result of some extensive testing? A good change anyway :)

+  // we need to convert the attribute array, which is alligned with the
multibyte text into an array of offsets

alligned -> aligned

Comment 4

[Dainis Jonitis]

Attachment: diff -d -u -8

Comment 5

[Dainis Jonitis]

> its -> it's

Fixed


> +NS_METHOD nsWindow::Resize(PRInt32 aX, PRInt32 aY, PRInt32 aWidth, PRInt32
> aHeight, PRBool  aRepaint)
>
> Extra space before aRepaint.

Fixed


>-  //}
>
>This in SetCursor belongs to the commented-out if in the beginning. I'd leave
>it in. 

Somehow I missed the matching if - fixed.


>-	 }
>-	 break;
>+    } break;
>
>I don't think this is a good change. The current style is predominant and good.

Changed all places to have bracket in one line and break in next one. Also
changed those lines that had an opening bracket after case to have it in
separate line.


>-	       if (newWidth != mLastSize.width)
>+	 if (newWidth > mLastSize.width)
>
>Are you sure this doesn't break anything if a window gets narrower? 

At least from browsing a couple of pages I have not noticed any diffeence. It
should not break anything because invalidating negative width/height does not
perform any real task anyway.


>-	 for (x = 0; x < 1000000; x++);
>+      PR_Sleep(PR_MillisecondsToInterval(30));
>
>A good guess or a result of some extensive testing? A good change anyway :)

It was just a guess. But now I even verified that it looks fine.


> alligned -> aligned

Fixed

Comment 6

[Ere Maijala (slow)]

Patch checked in to trunk.