http://msdn.microsoft.com/en-us/library/dd378459%28v=VS.85%29.aspx http://msdn.microsoft.com/en-us/library/dd378430%28v=VS.85%29.aspx Seamonkey needs access to this ability to set individual app user model ids per top level application window.

-#include <propvarutil.h> -#include <propkey.h> + #include <propvarutil.h> + #include <propkey.h> (ignore that ^^)

Some simple example code that works in the ede javascript console: var wm = Cc["@mozilla.org/appshell/window-mediator;1"].getService(Ci.nsIWindowMediator); var win = wm.getMostRecentWindow("navigator:browser"); var taskbar = Cc["@mozilla.org/windows-taskbar;1"].getService(Ci.nsIWinTaskbar); // Split this window out into it's own stack taskbar.setGroupIdForWindow(win, "myId"); // Restore this window to the bottom of the default stack taskbar.setDefaultGroupIdForWindow(win);

Comment on attachment 443501 [details] [diff] [review] patch v.1 I'd like to see a slightly different API. First, it's not clear why the functions return a boolean since this is equivalent to NS_SUCCEEDED (actually you should always write a return value, even in error cases). Second, the SetDefaultGroupIdFromWindow function just grabs a window-invariant string and then essentially calls SetGroupIdForWindow - how about we expose this window-invariant string as a readonly property instead of having SetDefaultGroupIdFromWindow nit: no need to have the idl prototype in a comment 2 lines above the C++ definition GetHWNDFromDOMWindow can reuse a lot of GetHWNDFromDocShell which we already use elsewhere in this file. >+nsresult >+SetWindowAppUserModelProp(nsIDOMWindow *aParent, const nsString & aIdentifier) >+{ >... >+ PROPVARIANT pv; >+ if (FAILED(InitPropVariantFromString(aIdentifier.get(), &pv))) >+ return NS_ERROR_INVALID_ARG; >+ >+ IPropertyStore* pPropStore; >+ if (FAILED(SHGetPropertyStoreForWindow(toplevelHWND, IID_PPV_ARGS(&pPropStore))) || >+ FAILED(pPropStore->SetValue(PKEY_AppUserModel_ID, pv)) || >+ FAILED(pPropStore->Commit()) || >+ FAILED(pPropStore->Release())) { >+ PropVariantClear(&pv); >+ return NS_ERROR_FAILURE; >+ } >+ >+ PropVariantClear(&pv); >+ >+ return NS_OK; >+} If SetValue or Commit fail, we leak the property store. Can you wrap it in an nsCOMPtr for sanity? Also, Release returns a ULONG and not an HRESULT so applying FAILED is the wrong thing to do (the return value is for testing purposes only). Also I notice that you call PropVariantClear in both return paths - I don't know if you can combine them somehow but it would be a plus if you could (don't do it if it makes the code harder to read or slower).

(In reply to comment #5) > (From update of attachment 443501 [details] [diff] [review]) > I'd like to see a slightly different API. > > First, it's not clear why the functions return a boolean since this is > equivalent to NS_SUCCEEDED (actually you should always write a return value, > even in error cases). Good point, a try catch and docs on exceptions would suffice. > Second, the SetDefaultGroupIdFromWindow function just grabs a window-invariant > string and then essentially calls SetGroupIdForWindow - how about we expose > this window-invariant string as a readonly property instead of having > SetDefaultGroupIdFromWindow > This is available through the new taskbar info interface. Creating two interfaces though for this and grabbing a string to set a string.. is cumbersome. The setdefault takes care of this. I'd prefer to keep it. > nit: no need to have the idl prototype in a comment 2 lines above the C++ > definition Hmm. I've always added that, although I'm not seeing anything in the style guidelines. Isn't this standard practice? > > GetHWNDFromDOMWindow can reuse a lot of GetHWNDFromDocShell which we already > use elsewhere in this file. > > >+nsresult > >+SetWindowAppUserModelProp(nsIDOMWindow *aParent, const nsString & aIdentifier) > >+{ > >... > >+ PROPVARIANT pv; > >+ if (FAILED(InitPropVariantFromString(aIdentifier.get(), &pv))) > >+ return NS_ERROR_INVALID_ARG; > >+ > >+ IPropertyStore* pPropStore; > >+ if (FAILED(SHGetPropertyStoreForWindow(toplevelHWND, IID_PPV_ARGS(&pPropStore))) || > >+ FAILED(pPropStore->SetValue(PKEY_AppUserModel_ID, pv)) || > >+ FAILED(pPropStore->Commit()) || > >+ FAILED(pPropStore->Release())) { > >+ PropVariantClear(&pv); > >+ return NS_ERROR_FAILURE; > >+ } > >+ > >+ PropVariantClear(&pv); > >+ > >+ return NS_OK; > >+} > > If SetValue or Commit fail, we leak the property store. Can you wrap it in an > nsCOMPtr for sanity? Also, Release returns a ULONG and not an HRESULT so > applying FAILED is the wrong thing to do (the return value is for testing > purposes only). Also I notice that you call PropVariantClear in both return > paths - I don't know if you can combine them somehow but it would be a plus if > you could (don't do it if it makes the code harder to read or slower). Good catch, will clean this up.

Comment on attachment 443911 [details] [diff] [review] patch v.2 comments addressed.

Comment on attachment 443911 [details] [diff] [review] patch v.2 Would be nice if we didn't have to copy the string into the nsString but it seems that's not possible in this case.

Comment on attachment 443911 [details] [diff] [review] patch v.2 sr? -> roc for interface changes.

http://hg.mozilla.org/mozilla-central/rev/44dd40341996 Seamonky folks, you're good to go! I posted some sample script lower in the bug.

(In reply to comment #11) > Seamonky folks, you're good to go! I posted some sample script lower in the > bug. jimm, thanks a lot for the groundwork here, we appreciate that very much!