Re: Taskbar grouping in Windows 7

[Jon TURNEY <jon.turney@dronecode.org.uk>]

On 25/06/2011 13:48, Tobias Häußler wrote:
> I created a small patch for XWin that adds correct grouping of taskbar
> icons when 'Always combine, hide labels' is set in the taskbar
> properties. It uses the new taskbar APIs introduced in Windows 7 to set
> an application id for each window. The id is based on the X11 class hints.
> Maybe it is useful for someone...

Firstly, thanks very much for this patch.

Getting Windows to correctly group XWin windows on the taskbar is something
that has needed fixing for a while, so it's great to have it done :-)

A few minor comments included inline:

> diff -urN src/xserver-cygwin-1.10.1-1/hw/xwin/taskbar.h
src/xserver-cygwin-1.10.1-1/hw/xwin/taskbar.h
> --- src/xserver-cygwin-1.10.1-1/hw/xwin/taskbar.h	1970-01-01
01:00:00.000000000 +0100
> +++ src/xserver-cygwin-1.10.1-1/hw/xwin/taskbar.h	2011-06-25
14:08:15.768141100 +0200

Can you start this file with the license information, like all other files.

This should also serve to confirm that you are happy for me to forward this
upstream to be distributed under the MIT/X11 license [1]

> @@ -0,0 +1,44 @@
> +#ifndef _TASKBAR_H
> +#define _TASKBAR_H
> +
> +#include <winnt.h>
> +#include <wingdi.h>
> +#include <objbase.h>
> +#include <initguid.h>
> +
> +typedef struct _tagpropertykey {
> +    GUID fmtid;
> +    DWORD pid;
> +} PROPERTYKEY;
> +#define REFPROPERTYKEY const PROPERTYKEY *
> +#define REFPROPVARIANT const PROPVARIANT *
> +
> +#ifdef INTERFACE
> +#undef INTERFACE
> +#endif
> +
> +#define INTERFACE IPropertyStore
> +DECLARE_INTERFACE_(IPropertyStore,IUnknown)
> +{
> +	STDMETHOD(QueryInterface)(THIS_ REFIID,PVOID*) PURE;
> +	STDMETHOD_(ULONG,AddRef)(THIS) PURE;
> +	STDMETHOD_(ULONG,Release)(THIS) PURE;
> +	STDMETHOD(GetCount)(THIS_ DWORD) PURE;
> +	STDMETHOD(GetAt)(THIS_ DWORD,PROPERTYKEY) PURE;
> +	STDMETHOD(GetValue)(THIS_ REFPROPERTYKEY,PROPVARIANT) PURE;
> +	STDMETHOD(SetValue)(THIS_ REFPROPERTYKEY,REFPROPVARIANT) PURE;
> +	STDMETHOD(Commit)(THIS) PURE;
> +};
> +#undef INTERFACE
> +typedef IPropertyStore *LPPROPERTYSTORE;
> +
> +DEFINE_GUID(IID_IPropertyStore,0x886d8eeb, 0x8cf2, 0x4446, 0x8d,0x02,
0xcd,0xba,0x1d,0xbd,0xcf,0x99);
> +
> +#define DEFINE_PROPERTYKEY(name, l, w1, w2, b1, b2, b3, b4, b5, b6, b7, b8,
pid) const PROPERTYKEY DECLSPEC_SELECTANY name = { { l, w1, w2, { b1, b2,  b3,
 b4,  b5,  b6,  b7,  b8 } }, pid }
> +
> +DEFINE_PROPERTYKEY(PKEY_AppUserModel_ID, 0x9F4C2855, 0x9F79, 0x4B39, 0xA8,
0xD0, 0xE1, 0xD4, 0x2D, 0xE1, 0xD5, 0xF3, 5);
> +
> +typedef HRESULT (__stdcall
*SHGETPROPERTYSTOREFORWINDOWPROC)(HWND,REFIID,void**);
> +typedef HRESULT (__stdcall *PROPVARIANTCLEARPROC)(PROPVARIANT*);
> +
> +#endif
> diff -urN src/xserver-cygwin-1.10.1-1/hw/xwin/winmultiwindowwm.c
src/xserver-cygwin-1.10.1-1/hw/xwin/winmultiwindowwm.c
> --- src/xserver-cygwin-1.10.1-1/hw/xwin/winmultiwindowwm.c	2011-04-22
18:03:27.000000000 +0200
> +++ src/xserver-cygwin-1.10.1-1/hw/xwin/winmultiwindowwm.c	2011-06-25
14:11:05.671216100 +0200
> @@ -61,6 +61,9 @@
>  #include "window.h"
>  #include "pixmapstr.h"
>  #include "windowstr.h"
> +#include "taskbar.h"
> +
> +#include <shlwapi.h>
>
>  #ifdef XWIN_MULTIWINDOWEXTWM
>  #include <X11/extensions/windowswmstr.h>
> @@ -200,6 +203,9 @@
>  void
>  winUpdateWindowPosition (HWND hWnd, Bool reshape, HWND *zstyle);
>
> +void
> +winSetAppID (HWND hWnd, const char* AppID);
> +

Looks like this function can be made static?

>  /*
>   * Local globals
>   */
> @@ -1671,19 +1677,36 @@
>      XFree(normal_hint);
>    }
>
> -  /* Override hint settings from above with settings from config file */
> +  /*
> +    Override hint settings from above with settings from config file and set
> +    application id for grouping.
> +  */
>    {
>      XClassHint class_hint = {0,0};
>      char *window_name = 0;
> -
> +    char *application_id = 0;
> +
>      if (XGetClassHint(pDisplay, iWindow, &class_hint))
>        {
>          XFetchName(pDisplay, iWindow, &window_name);
>
>          style = winOverrideStyle(class_hint.res_name, class_hint.res_class,
window_name);
>
> +#define APPLICATION_ID_FORMAT	"%s.xwin.%s"
> +#define APPLICATION_ID_UNKNOWN "unknown"
> +        if (class_hint.res_class)
> +          {
> +            asprintf (&application_id, APPLICATION_ID_FORMAT, XVENDORNAME,
class_hint.res_class);
> +          }
> +        else
> +          {
> +            asprintf (&application_id, APPLICATION_ID_FORMAT, XVENDORNAME,
APPLICATION_ID_UNKNOWN);
> +          }
> +        winSetAppID (hWnd, application_id);
> +
>          if (class_hint.res_name) XFree(class_hint.res_name);
>          if (class_hint.res_class) XFree(class_hint.res_class);
> +        if (application_id) free(application_id);
>          if (window_name) XFree(window_name);
>        }
>      else
> @@ -1691,7 +1714,7 @@
>          style = STYLE_NONE;
>        }
>    }
> -
> +
>    if (style & STYLE_TOPMOST) *zstyle = HWND_TOPMOST;
>    else if (style & STYLE_MAXIMIZE) maxmin = (hint & ~HINT_MIN) | HINT_MAX;
>    else if (style & STYLE_MINIMIZE) maxmin = (hint & ~HINT_MAX) | HINT_MIN;
> @@ -1789,3 +1812,73 @@
>      winUpdateRgnMultiWindow(pWin);
>    }
>  }
> +
> +void
> +winSetAppID (HWND hWnd, const char* AppID)
> +{
> +  HMODULE hmodShell32Dll = NULL, hmodOle32Dll = NULL;
> +  SHGETPROPERTYSTOREFORWINDOWPROC pSHGetPropertyStoreForWindow = NULL;
> +  PROPVARIANTCLEARPROC pPropVariantClear = NULL;
> +  PROPVARIANT pv;
> +  IPropertyStore *pps = NULL;
> +  HRESULT hr;
> +
> +  hmodShell32Dll = LoadLibrary ("shell32.dll");
> +  if (hmodShell32Dll == NULL)
> +    {
> +      ErrorF ("winSetAppID - Could not load shell32.dll\n");
> +      return;
> +    }

Since the results of LoadLibrary/GetProcAddress are invariant at run-time, I'd
rather these calls were done once, rather than every time we want to use those
results.

It would also be useful to include a comment mentioning which version of
Windows these interfaces were added in (so future generations will know when
they can directly link to those functions :-))

I assume that there is no way to achieve this functionality prior for Windows
7, rather than an alternative interface which you chose not to use?

> +
> +  pSHGetPropertyStoreForWindow = (SHGETPROPERTYSTOREFORWINDOWPROC)
> +                                        GetProcAddress (hmodShell32Dll,
> +                                          "SHGetPropertyStoreForWindow");
> +  if (pSHGetPropertyStoreForWindow == NULL)
> +    {
> +      ErrorF ("winSetAppID - Could not get "
> +              "SHGetPropertyStoreForWindow address\n");
> +      FreeLibrary (hmodShell32Dll);
> +      return;
> +    }
> +
> +
> +  hmodOle32Dll = LoadLibrary ("ole32.dll");
> +  if (hmodOle32Dll == NULL)
> +    {
> +      ErrorF ("winSetAppID - Could not load ole32.dll\n");
> +      FreeLibrary (hmodShell32Dll);
> +      return;
> +    }
> +
> +  pPropVariantClear = (PROPVARIANTCLEARPROC)
> +                            GetProcAddress (hmodOle32Dll,
> +                              "PropVariantClear");
> +  if (pPropVariantClear == NULL)
> +    {
> +      ErrorF ("winSetAppID - Could not get "
> +              "PropVariantClear address\n");
> +      FreeLibrary (hmodShell32Dll);
> +      FreeLibrary (hmodOle32Dll);
> +      return;
> +    }
> +
> +  hr = pSHGetPropertyStoreForWindow (hWnd, &IID_IPropertyStore, (void**)&pps);
> +  if(SUCCEEDED(hr) && pps)
> +  {
> +    memset(&pv, 0, sizeof(PROPVARIANT));

Should be PropVariantInit() ?

> +    pv.vt = VT_LPWSTR;
> +    hr = SHStrDupA(AppID, &pv.pwszVal);
> +    if(SUCCEEDED(hr))
> +    {
> +      hr = pps->lpVtbl->SetValue(pps, &PKEY_AppUserModel_ID, &pv);
> +      if (pPropVariantClear != NULL )
> +        {
> +          pPropVariantClear(&pv);
> +        }
> +    }
> +    pps->lpVtbl->Release(pps);
> +  }

The MSDN description of SHGetPropertyStoreForWindow() says:
"A window's properties must be removed before the window is closed. If this is
not done, the resources used by those properties are not returned to the system."

Does this not apply in this case, or is some cleanup needed?

> +
> +  FreeLibrary (hmodOle32Dll);
> +  FreeLibrary (hmodShell32Dll);
> +}

[1] http://cgit.freedesktop.org/xorg/xserver/tree/COPYING

-- 
Jon TURNEY
Volunteer Cygwin/X X Server maintainer

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://x.cygwin.com/docs/
FAQ:                   http://x.cygwin.com/docs/faq/