After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 763851 - GDK W32: show_window_menu() is not implemented
GDK W32: show_window_menu() is not implemented
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Win32
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-win32 maintainers
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-03-18 04:59 UTC by LRN
Modified: 2016-03-29 14:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GDK W32: Implement show_window_menu() (5.78 KB, patch)
2016-03-18 05:00 UTC, LRN
none Details | Review
GDK W32: Implement show_window_menu() (5.74 KB, patch)
2016-03-18 05:04 UTC, LRN
none Details | Review
GDK W32: Implement show_window_menu() (6.96 KB, patch)
2016-03-18 05:20 UTC, LRN
none Details | Review
GDK W32: Implement show_window_menu() (7.53 KB, patch)
2016-03-18 08:24 UTC, LRN
none Details | Review
GDK W32: Implement show_window_menu() (7.43 KB, patch)
2016-03-26 00:58 UTC, LRN
none Details | Review
GDK W32: Implement show_window_menu() (7.73 KB, patch)
2016-03-29 13:53 UTC, LRN
committed Details | Review

Description LRN 2016-03-18 04:59:52 UTC
This results in GTK using fallback window popup menu,
which is very feature-poor (even if its implementation is
improved).
Comment 1 LRN 2016-03-18 05:00:02 UTC
Created attachment 324229 [details] [review]
GDK W32: Implement show_window_menu()

This is achieved by sending undocumented message 0x313
to the window.
Before doing that, the window is given WS_SYSMENU style
(to enable window menu) and some combination of
WS_MAXIMIZEBOX (for "Mazimize" item)
WS_MINIMIZEBOX (for "Minimize" item)
WS_SIZEBOX (for "Size" item)
depending on which operations are currently permissible.

0x313 is processed by DefWindowProc(), which results
in showing the window menu. We remove extra styles
at the first opportunity (WM_INITMENU message), as they
alter the way our window is rendered.
Comment 2 LRN 2016-03-18 05:04:57 UTC
Created attachment 324230 [details] [review]
GDK W32: Implement show_window_menu()

v2:
* Use this for non-layered windows as well

Now, the last thing after this is to see how it works for non-CSD windows.
Comment 3 LRN 2016-03-18 05:20:44 UTC
Created attachment 324231 [details] [review]
GDK W32: Implement show_window_menu()

v3:
* Make it work more or less correctly on non-CSD windows
  (see comments in the code for the description of some
   corner cases that are probably handled incorrectly).
Comment 4 Fan, Chun-wei 2016-03-18 06:45:23 UTC
Review of attachment 324231 [details] [review]:

Hi LRN,

Please see my comments about this.

::: gdk/win32/gdkevents-win32.c
@@ +1910,3 @@
+static gboolean
+handle_0x313 (GdkWindow *window, MSG *msg, gint *ret_valp)
+{

For the 0x313, since that is undocumented, it means that MS might change this value at some point, and 0x313 is not a value that is that easy to remember what it really does.  Perhaps it would be better if we define a constant/variable (that is easier to remember for this case) to 0x313, and add the new value(s) (or enum) when the value is changed or is designated a enum.
Comment 5 LRN 2016-03-18 08:24:30 UTC
Created attachment 324238 [details] [review]
GDK W32: Implement show_window_menu()

v4:
* 0x313 -> WM_SYSMENU
* Set have_temp_styles to FALSE at the end of handle_wm_sysmenu()
* Nicer WM_INITMENU handling (easier to extend later)
Comment 6 LRN 2016-03-21 08:59:08 UTC
This patch has a bug - if you rightclick not on the headerbar, but on a button inside of the headerbar, menu is not positioned correctly (i.e. the code confuses the offset from topleft of a widget and the offset from topleft of a window).
Comment 7 LRN 2016-03-26 00:58:37 UTC
Created attachment 324778 [details] [review]
GDK W32: Implement show_window_menu()

v5:
* Fixed a bug: uses root event coords instead of getting event coords
  and then trying to rootify them.
Comment 8 Matthias Clasen 2016-03-26 15:47:02 UTC
If we define a constant ourselves, it would be better to use a prefix go avoid clash with constants ms might define in the future
Comment 9 LRN 2016-03-26 16:24:03 UTC
WM_SYSMENU is not a random name. It was pried out of some MS table matches message names and codes[1]. I don't expect it to change, and if some headers do define it, they will define it to this value.

[1] http://blog.airesoft.co.uk/2009/11/wm_messages/
Comment 10 Paolo Borelli 2016-03-29 13:30:39 UTC
Review of attachment 324778 [details] [review]:

::: gdk/win32/gdkprivate-win32.h
@@ -98,2 +98,3 @@
 #endif
 
+#ifndef WM_SYSMENU

Would be good to have a link to some kind of reference albeit unofficial

::: gdk/win32/gdkwindow-win32.h
@@ -175,1 +180,5 @@
   guint suppress_layered;
+
+  /* Temporary styles that this window got for the purpose of
+   * handling WM_SYSMENU.
+   * They are removed at the opportunity (usually WM_INITMENU).

I guess you meant "first opportunity"
Comment 11 LRN 2016-03-29 13:53:19 UTC
Created attachment 324937 [details] [review]
GDK W32: Implement show_window_menu()

v6:
* Fixed a missing word in a comment
* Added a comment about WM_SYSMENU, with links to the sources of information
  about WM_SYSMENU.
Comment 12 LRN 2016-03-29 14:30:13 UTC
Since all nitpicks were fixed, and no one gave any other objections,
attachment 324937 [details] [review] pushed as cea8c29 - GDK W32: Implement show_window_menu()