When the user launches a native app, the correct app icon is shown in the top left corner of the window (frame), when they close the app, the app icon is replaced with the Firefox icon until the app is closed. This occurs with Windows 7, Vista and presumably XP (but cannot verify as app icon support is still broken). Steps to Reproduce: 1. Launch Firefox - Make sure to use FF9 or greater 2. Install the Apps extension - openwebapps-23dc5022e9-12_02_2011.xpi from http://people.mozilla.com/~dclarke/openwebapps/Extension/ 3. Go to apps.mozillalabs.com/appdir 4. Install an app - Make sure to choose 'Install (with Native app)' 5. Double click on the desktop icon to launch the app - Notice correct app icon is shown on the top right corner of the window (frame) 6. Close the app and make sure to notice the app icon on the top left corner of the window (frame) Actual Results: The app icon is replaced with the Firefox icon while the app is closing Expected Results: The Firefox icon should not replace the app icon at any time in the window (frame)

A Pivotal Tracker story has been created for this Bug: http://www.pivotaltracker.com/story/show/24808727

Definitely mozilla-central worthy.

I think we establish what to do with the existing tracking bug. If it's going to be used, then this bug still needs to block the tracking bug.

On the new mozilla-central implementation on Win 7, I don't see the firefox icon appear on close, but I do see a different icon appearing than the app icon on close. The icon appearing on close looks like the icon for an executable for windows. This bug is still valid, except its a different icon being shown.

I'm unable to reproduce this issue on latest Nightly. Is this still an issue?

(In reply to Tim Abraldes from comment #5) > I'm unable to reproduce this issue on latest Nightly. Is this still an > issue? Yes. Just re-tested it. Try this test case: 1. Go to apps.mozillalabs.com/appdir 2. Install Favimon 3. Launch Favimon 4. Watch the icon in the top left of the window - Close the app window You should see the favimon icon switch from that icon to the executable icon. Note - Does not look like this happens with every single application. Screenshot of issue: http://screencast.com/t/XuvoxPxUbF

Discussed this with rs: It's possible that simply hiding the window before the icon resources get removed/destroyed would solve this issue.

I meant to add this link [https://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp?rev=f4157e8c4107#6874]. This is where we would want to hide the window.

The attached patch fixes this issue in my local testing. Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=670127177d1e Jimm: Do you see any danger with this proposed patch?

Isn't this going to get rid of the nice fade out windows experience on Vista+? A better solution might be to move the DestroyIcon code to the OnDestroy() handler. According to msdn windows that receive WM_DESTROY have already been removed from the screen.

(In reply to Jim Mathies [:jimm] from comment #10) > Isn't this going to get rid of the nice fade out windows experience on > Vista+? A better solution might be to move the DestroyIcon code to the > OnDestroy() handler. According to msdn windows that receive WM_DESTROY have > already been removed from the screen. Oh, pfft, never mind, it's already in OnDestroy(). This is weird, I don't see this problem with Fx, I'm guessing that because we don't use SetIcon and web apps do.

My main concern with the hide call is that it will likely fire gecko events, which I'd like to avoid. How about caching the two icons we set in a member variable and free them in the dtor?

This patch uses jimm's suggestion (in `nsWindow::OnDestroy` cache the window's big and small icon, then in `nsWindow::~nsWindow` free them). The member variables could probably use better names, but this patch works in local testing. What do you think, Jim?

Comment on attachment 637328 [details] [diff] [review] Patch v2 Review of attachment 637328 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/windows/nsWindow.cpp @@ +6953,5 @@ > } > > + // Store icon resources to be freed > + mIconTempBig = (HICON) ::SendMessageW(mWnd, WM_GETICON, (WPARAM)ICON_BIG, (LPARAM) 0); > + mIconTempSmall = (HICON) ::SendMessageW(mWnd, WM_GETICON, (WPARAM)ICON_SMALL, (LPARAM) 0); Instead of doing this in OnDestroy, why not store the icons in the member variables when they are set in SetIcon?

This patch initializes the member variables (patch v2 omitted the member initialization) and stores the icons during `nsWindow::SetIcon()` rather than during `nsWindow::OnDestroy` This patch is running through tryserver: https://tbpl.mozilla.org/?tree=Try&rev=031d0e649054 (In reply to Jim Mathies [:jimm] from comment #14) > Comment on attachment 637328 [details] [diff] [review] > Patch v2 > > Review of attachment 637328 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: widget/windows/nsWindow.cpp > @@ +6953,5 @@ > > } > > > > + // Store icon resources to be freed > > + mIconTempBig = (HICON) ::SendMessageW(mWnd, WM_GETICON, (WPARAM)ICON_BIG, (LPARAM) 0); > > + mIconTempSmall = (HICON) ::SendMessageW(mWnd, WM_GETICON, (WPARAM)ICON_SMALL, (LPARAM) 0); > > Instead of doing this in OnDestroy, why not store the icons in the member > variables when they are set in SetIcon? I was trying to avoid edge cases where someone uses `WM_SETICON` to set our window's icon without our knowledge (i.e. not through `nsWindow::SetIcon`): If we get the icon during `OnDestroy()` we can be sure that we're destroying the right icon. On the other hand, our icon should only ever be set through `nsWindow::SetIcon()` so that might be overly cautious. I've updated the patch to set the member variables during `nsWindow::SetIcon()`

Tried with: - Windows 7 - Firefox Nightly Build identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0 (updated on July 6, 2012) Installed the following apps from apps.mozillalabs.com/appdir: - Favimon (beta) - BarFight - Lucidchart Also installed from https://marketplace.mozilla.org/en-US/ (apps-preview): - Jauntly - Galactians 2 Launched apps from desktop and closed each one and the problem does not occur anymore. The app icon remains in the window frame until the app closes. Tried with having FF nightly and beta opened while the app was running and without having any FF open and worked in all cases. Also tried with windows maximized and still no issue appeared. The issue seems fixed but I only tested on Window 7 as that's all I have. I'll keep the status as is since testing on Vista and XP still needs to be done.

Thanks Mohamed. I finished the testing on this for the other pieces. Marking as verified.