Created attachment 316041 [details] [review]
GDK W32: Don't use SetWindowLong() to set/unset WS_EX_TOPMOST

While searching for bug 746745 it was discovered that one could
not set WS_EX_TOPMOST extended window style with SetWindowLong(),
but must use SetWindowPos() for that purpose.

This was never a problem most likely because it is highly unlikely for windows
to acquire/lose WS_EX_TOPMOST after they are created, by means other
than SetWindowPos() (which GTK does use to raise/lower windows and
set/remove keep_above).

Review of attachment 316041 [details] [review]:

See the comments.

::: gdk/win32/gdkwindow-win32.c
@@ +2462,3 @@
 
+      SetWindowLong (GDK_WINDOW_HWND (window), GWL_EXSTYLE,
+		     new_exstyle & (~WS_EX_TOPMOST));

so, why is new_style having this flag set? should we ensure that we never get it here?

@@ +2477,3 @@
+  if ((new_style & WS_EX_TOPMOST) &&
+      !(old_style & WS_EX_TOPMOST))
+  {

indentation is wrong

(In reply to Ignacio Casal Quinteiro (nacho) from comment #2)
> Review of attachment 316041 [details] [review] [review]:
> 
> ::: gdk/win32/gdkwindow-win32.c
> @@ +2462,3 @@
>  
> +      SetWindowLong (GDK_WINDOW_HWND (window), GWL_EXSTYLE,
> +		     new_exstyle & (~WS_EX_TOPMOST));
> 
> so, why is new_style having this flag set? should we ensure that we never
> get it here?
>

That is certainly possible, but might take more code (although with the latest iteration of this patch it might already be taking too much code anyway, so maybe that would be better).

Is that all?

yeah, I did not spot anything else. Feel free to make the changes.

Created attachment 316307 [details] [review]
GDK W32: Don't use SetWindowLong() to set/unset WS_EX_TOPMOST

V2:
Remove WS_EX_TOPMOST from exstyle

The testing i've done indicates that it's not strictly necessary to do so,
but it's probably cleaner (and more readable) that way

Review of attachment 316307 [details] [review]:

Double check the indentation since it seems wrong.

::: gdk/win32/gdkwindow-win32.c
@@ +2428,2 @@
   if (window->window_type == GDK_WINDOW_TEMP)
+  {

indentation

Created attachment 316314 [details] [review]
GDK W32: Don't use SetWindowLong() to set/unset WS_EX_TOPMOST

V3:
Changed indentation to use the same brainded scheme used by the surrounding code.

Review of attachment 316314 [details] [review]:

still indentation issues.

::: gdk/win32/gdkwindow-win32.c
@@ +2433,2 @@
   else if (impl->type_hint == GDK_WINDOW_TYPE_HINT_UTILITY)
+   {

missing one space this indentation

@@ +2490,2 @@
+  if (will_be_topmost && !was_topmost)
+  {

here is missing the 2 spaces

@@ +2493,3 @@
+  }
+  else if (was_topmost && !will_be_topmost)
+  {

same here

@@ +2497,3 @@
+  }
+  else
+  {

and here

Created attachment 316316 [details] [review]
GDK W32: Don't use SetWindowLong() to set/unset WS_EX_TOPMOST

V4:
Even more braindead indentation.

Review of attachment 316316 [details] [review]:

Patch seems fine, but should you update the commit message? Feel free to update it and push it.

Attachment 316316 [details] pushed as db1b242 - GDK W32: Don't use SetWindowLong() to set/unset WS_EX_TOPMOST