search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

Pinned Icon becomes invalid after update #12471

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by grappler Saturday Dec 21, 2013 at 09:31 GMT Originally opened as https://github.com/adobe/brackets/issues/6297

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.

core-ai-bot commented 3 years ago

Comment by peterflynn Sunday Dec 22, 2013 at 04:01 GMT

Tentatively setting medium priority@bchintx

core-ai-bot commented 3 years ago

Comment by le717 Wednesday Feb 12, 2014 at 17:30 GMT

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 %ProgramFiles%\Brackets. Since the path and exe are the same each update, I can't seem to figure out what could be causing this.

core-ai-bot commented 3 years ago

Comment by peterflynn Wednesday Feb 12, 2014 at 18:20 GMT

@grappler@le717 Can you check this?

  1. Right-click the 'broken' pinned icon
  2. You should see a menu with two entries -- Brackets, and Unpin. Right-click the Brackets entry.
  3. Choose Properties
  4. What do the "Target" and "Start in" fields say?
core-ai-bot commented 3 years ago

Comment by le717 Wednesday Feb 12, 2014 at 18:27 GMT

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.

core-ai-bot commented 3 years ago

Comment by le717 Thursday Feb 13, 2014 at 01:58 GMT

@peterflynn Got the screenshots you asked for.

Shortcut with Sprint 35 installed: sprint 35 icon

Shortcut with Sprint 36 installed over Sprint 35: sprint 36 icon

core-ai-bot commented 3 years ago

Comment by le717 Thursday Feb 13, 2014 at 02:41 GMT

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...

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 13, 2014 at 21:14 GMT

@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?

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 13, 2014 at 21:19 GMT

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?

  1. Open command prompt
  2. cd to folder containing the installer MSI file
  3. Run msiexec /i brackets-sprint-36-WIN.msi /l*v brackets_log.txt
  4. When the installer is done, upload the log file as a Gist and paste a link here. (Or if you're worried about privacy, you can email it directly to me -- pflynn at adobe).
core-ai-bot commented 3 years ago

Comment by grappler Thursday Feb 13, 2014 at 21:28 GMT

@peterflynn Here you go... https://gist.github.com/grappler/1b80f1c80d5d64f04238

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 13, 2014 at 21:32 GMT

@le717 Can you do the same logging step? Thanks!

And@grappler can you follow my steps above to see if the path the shortcut changes to is consistent with what@le717 has reported above? Thanks to you as well!

core-ai-bot commented 3 years ago

Comment by grappler Thursday Feb 13, 2014 at 21:43 GMT

@peterflynn Yes, it is the same as@le717. The problem that I see is that the icon is not anymore in C:\Users\grapplerulrich\AppData\Roaming\Microsoft\Internet Explorer\Quick Launch\User Pinned\TaskBar which is the location before and after the update.

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Feb 13, 2014 at 21:49 GMT

@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?

core-ai-bot commented 3 years ago

Comment by grappler Thursday Feb 13, 2014 at 21:52 GMT

@peterflynn The screenshots match what I see. When I access the Location: C:\Users\grapplerulrich\AppData\Roaming\Microsoft\Internet Explorer\Quick Launch\User Pinned\TaskBar the Brackets icon is deleted after the update.

core-ai-bot commented 3 years ago

Comment by grappler Thursday Feb 13, 2014 at 21:54 GMT

This is a screenshot of the general tab of a working link/shortcut/pinned icon image

core-ai-bot commented 3 years ago

Comment by le717 Thursday Feb 13, 2014 at 22:41 GMT

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 C:\Users\UserName\AppData\Roaming\Microsoft\Internet Explorer\Quick Launch\User Pinned\TaskBar) is getting deleted by the Sprint 36 installer. On a working shortcut, a Shortcut"tab (as seen in mine and@grappler's screenshots) is present, indicating a shortcut is present to Brackets at the above folder location. The Sprint 36 installer is apparently deleting that shortcut. What then happens is Windows still tries to find and use the now-nonexistent shortcut. Thus occurs my second screenshot where the Shortcut tab is missing.

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 User Pinned\TaskBar) in 7 to prevent developers from blatantly dropping a shortcut in the quick launch without an option to not do that (in short, it was being abused). So starting with 7, they changed the folder location so these abusive installers could no longer do that. Rather, all shortcuts to be pinned must occur by user interaction, AKA right-click > Pin to Taskbar. That is how I created my quick launch shortcut, and I'm sure that is how @grappler did it too, seeing the Brackets installer does not make a shortcut even in the old location.

As Brackets uses an MSI installer, it would seem MS has added a default action to check if a shortcut (located at C:\Users\UserName\AppData\Roaming\Microsoft\Internet Explorer\Quick Launch\User Pinned\TaskBar) that matches the app ID is present, and delete it if so. From their standpoint, it makes sense to do this as their developer documentation says quick launch was removed in Win 7.

core-ai-bot commented 3 years ago

Comment by le717 Thursday Feb 13, 2014 at 22:49 GMT

@peterflynn Here is the log you asked for. https://gist.github.com/le717/8985499

core-ai-bot commented 3 years ago

Comment by le717 Thursday Feb 13, 2014 at 22:54 GMT

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).

core-ai-bot commented 3 years ago

Comment by le717 Friday Feb 14, 2014 at 01:39 GMT

@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. :(

core-ai-bot commented 3 years ago

Comment by le717 Wednesday Mar 19, 2014 at 15:35 GMT

@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 https://github.com/adobe/brackets/issues/6297#issuecomment-35035371 is why this is happening and https://github.com/adobe/brackets/issues/6297#issuecomment-35036676 would be a possible, if not the fix for it.

core-ai-bot commented 3 years ago

Comment by peterflynn Wednesday Mar 19, 2014 at 22:03 GMT

@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...

core-ai-bot commented 3 years ago

Comment by le717 Thursday Mar 20, 2014 at 02:49 GMT

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 ;)).

core-ai-bot commented 3 years ago

Comment by bchintx Thursday Mar 20, 2014 at 22:42 GMT

Still able to reproduce on Win 8.1 using Sprint 37.

core-ai-bot commented 3 years ago

Comment by dalcala Thursday Mar 20, 2014 at 23:13 GMT

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.

core-ai-bot commented 3 years ago

Comment by dangoor Wednesday Apr 23, 2014 at 16:20 GMT

Reviewed. Keeping 1.0 milestone.

core-ai-bot commented 3 years ago

Comment by le717 Friday Apr 25, 2014 at 04:18 GMT

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 <Condition Message="Win7 or 2008 R2 required"><![CDATA[Installed OR VersionNT >= 601]]></Condition> (http://stackoverflow.com/a/2733980) is run. This checks if the installer is running on 7 or higher (as I said, the only OSs it effects (http://msdn.microsoft.com/en-us/library/windows/desktop/aa370556%28v=vs.85%29.aspx)). If it passes, then a <DirectorySearch> to%AppData%\Microsoft\Internet Explorer\Quick Launch\User Pinned\TaskBar, then a <FileSearch> for Brackets.lnk. (http://stackoverflow.com/a/7357806, http://madtechnology.wordpress.com/2007/05/04/finding-files-using-wix/) It then stores the boolean value (true) in a variable. If at any point in this search something does not validate (primarily if it is being on Vista), that variable is set to false. Late in the installation process (as I do not know exactly when the built-in code to remove an icon exists is run, this may take some trial and error to find the location, but it needs to be very late, as in close to finishing, because the built-in code needs to have already run), that variable value is checked. If it validates, then it goes back to the folder path searched and creates a Brackets.lnk file (<Shortcut>) using the installation path selected.

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.

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...

@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).

core-ai-bot commented 3 years ago

Comment by stowball Monday Jun 23, 2014 at 05:25 GMT

I'd appreciate a fix for this too as it affects me with each release. Thanks!

core-ai-bot commented 3 years ago

Comment by peterflynn Saturday Aug 16, 2014 at 01:36 GMT

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:

you can avoid the costs of minor upgrades by simply using major upgrades. You can remove files without worrying about component-rule violations if you use an “early” scheduling of the RemoveExistingProducts standard action...

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.