-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Pinned Icon becomes invalid after update #6297
Comments
Tentatively setting medium priority @bchintx |
I experienced this when updating from Sprint 35 to 36, Windows 8.1 Core x64. This is odd because (as you know) since Sprint 34 the installer by default installs to |
Sure thing. I'll have to remove 36 and reinstall 35 to test, but that's no problem (alternatively I could test on a Win 7 machine, but I'll stick to 8 right now). It won't be too soon, though. I'm in that ice storm hitting the southern US, so I'm using the GitHub Android app in case we loose power again. |
@peterflynn Got the screenshots you asked for. |
Furthermore, running Brackets while the broken shortcut is pinned makes the shortcut act like the running icon, and clicking it opens the Brackets window, rather than the expected action of a new icon with the Brackets logo being displayed. Oh great. I just realized I did not show the rest of the path before I somehow accidentally removed the broken icon. :\ Let me uninstall/reinstalled the Sprints again... |
@bchintx Is this a case where we'd benefit from getting a log when the upgrade installer is running? If so could you post instructions here? |
Oops, I think Bryan is out actually, but I think I've got the right steps here. @le717 when you run the sprint 36 installer the next time, can you do it this way?
|
@peterflynn Here you go... https://gist.github.com/grappler/1b80f1c80d5d64f04238 |
@peterflynn Yes, it is the same as @le717. The problem that I see is that the icon is not anymore in |
@grappler Could you clarify that a bit? I'm confused about "the icon is not anymore in" vs. "is the location before and after the update." Does what you see match the screenshots above? In other words, the "Target" field points at Brackets.exe beforehand (not the Quick Launch path), but afterward the "Target" field is gone and in its place there's a "Location" field pointing to Quick Launch? |
@peterflynn The screenshots match what I see. When I access the |
I understand what @grappler is talking about. I've written enough Inno Setup-based installers to understand. What seems to be happening is the quick launch shortcut (located at To recreate this yourself in an easier manner, go to the folder above and look at the quick launch ("pinned") shortcuts present. Rename one of the shortcuts, and proceed to click that shortcut. It will report as not found, and Windows will replace the icon with the generic page icon. As for why this is occurring is partly because of the changes Microsoft made in Windows 7. I cannot seem to find the article again, but the old location of quick launch ("Taskbar") shortcuts was changed (by appending As Brackets uses an MSI installer, it would seem MS has added a default action to check if a shortcut (located at |
@peterflynn Here is the log you asked for. |
A possible fix for this issue would be to add a section in the installer to not delete the shortcut if 1. this is Win7 or higher and 2. if the shortcuts even exists (OS version check would come first since the path does not exist on XP and Vista). |
@peterflynn @grappler Apologies for that large reply. I am an analyst (or at least am a very analytical thinker), so when I have an theory about something I tend to think it all the way through, creating long messages like the one above. :( |
@peterflynn @grappler I experienced this again yesterday when I updated from Sprint 36 to 37. I have written Inno Setup based installers before (see le717/Project-Risen), and I can say with almost complete surety #6297 (comment) is why this is happening and #6297 (comment) would be a possible, if not the fix for it. |
@le717 The installer source code is here: https://github.com/adobe/brackets-shell/tree/master/installer/win. Afaik there's no code that tries to either create or delete a Quick Launch shortcut, and MSI/Wix doesn't typically try to remove anything that there doesn't exist a creation step for. But if you spot something that looks off in that code, we'd be happy to try out a proposed change or PR... |
I am looking up relevant WiX functions and running some ideas in my mind right now. I'll get back to you this ASAP (I've got that other PR you reviewed to work on ;)). |
Still able to reproduce on Win 8.1 using Sprint 37. |
I saw this issue as well on Win 7 x64. When I installed Brackets Sprint 37, I lost my pinned Brackets icon from the Win 7 taskbar. It turned into a white generic icon in the taskbar and wouldn't launch Brackets. When I rebooted my machine, the white icon was gone completely from the taskbar and I had to manually re-pin Brackets to the taskbar. |
Reviewed. Keeping 1.0 milestone. |
OK, I have a rough idea how this can be fixed. Because I lost some of my resource bookmarks and I haven't looked into this in a month it is not as solid as I wanted. Consider it a first sketch. :) Very early on in the installer process (if possible, initialization would do), something to the effect of an This howto on the official WiX site might help put into code some of what I have attempted to explain, and may even be able to be adapted to work for the quick launch. If the thought of this evading the Microsoft installer guidelines is bugging you, remember that Windows pretty much forces pinned icons to be created by the user, not through automation. That's the reason the DirectorySearch and FileSearch is present: having faith the icon was at one point added by the user (which would true even a few sprints later, as it was originally created by them), it recreates the icon. If it does not exist, then it is not created. This way, the guidelines are still followed by not planting an icon in the taskbar/quick launch (as I explained in my huge comment above). Your only other options would be to deal with this bug or roll a new installer not based on Windows Installer, such as Inno Setup or NSIS. While they themselves do not create icons in the new location for 7+, they can be made to do so, and usually it is kinda simple to do such.
@peterflynn No, I do not see anything in any off the installer code that would neither create nor remove a quick launch icon, and no, Windows Installer does not usually perform actions it has not creation steps for (one of the advantages of using MSIs). However, because the quick launch path has undeniable changed in Windows 7 onward (did I mention the User Pinned folder has the hidden attribute applied to it?), that's what makes me believe there is code present in WiX/Win Installer itself that removes the shortcut). |
I'd appreciate a fix for this too as it affects me with each release. Thanks! |
Reviewed -- reducing priority but keeping in 1.0 to see if we have time to investigate more. Some links to similar issues in other apps:
The last link is the most interesting one to me: it draws a connection between the Start menu shortcut (which is created by the installer) and the Taskbar pinned icon (which is not created by the installer but is affected by it anyway). So removing the Start menu shortcut may be automatically removing the Taskbar pin too, since the OS sees them as connected. The answer at the bottom says that moving RemoveExistingProducts later in the upgrade sequence ("immediately before or after InstallFinalize") may fix this -- but the article it links to implies this means upgrading can never remove any files that were present in an earlier version. It's a little ambiguous on whether that caveat applies only to "minor upgrades," but it sure looks like it applies to all:
If that's true, then this is a fundamental limitation of MSI installers. @le717's last suggestion is a potential workaround -- recreating the .lnk file at the end if one used to exist before the upgrade started -- but I'm honestly a little skeptical that would work given how locked-down Taskbar pinning is supposed to be now... it's supposed to be impossible for an app to add itself programmatically, and the folder containing the shortcuts is described as a "cache," not the actual representation of what's pinned. |
I have brackets pinned to my taskbar on Windows 8 and when I update the icon breaks and I have to readd the icon to the taskbar.
The text was updated successfully, but these errors were encountered: