Created attachment 617727 [details]
App installation not allowed
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:14.0) Gecko/20120420 Firefox/14.0a1
Steps to reproduce:
1) Ensure that you have /Applications/Chess.app installed
2) Load https://marketplace-dev.allizom.org/en-US/app/chess-2/
3) Click the "Install" button and click "Install" in the doorhanger installer.
4) Notice the error: "App install not allowed" (error code: DENIED)
Because I'm on Mac OS X and Chess.app is already an application I have installed (as a native app, not a web app), it thinks the Chess webapp is already installed and rejects installation.
Confirmed on OS X 10.6.8. Also checked if you could install the app specified generally without a name conflict - That worked. Will want to see what happens on Windows as well to find out if this is OS-specific or not.
Timestamp: 4/23/12 6:29:30 PM
Error: Error installing app: [Exception... "Component returned failure code: 0x80520008 (NS_ERROR_FILE_ALREADY_EXISTS) [nsILocalFile.create]" nsresult: "0x80520008 (NS_ERROR_FILE_ALREADY_EXISTS)" location: "JS frame :: resource:///modules/WebappsInstaller.jsm :: <TOP_LEVEL> :: line 190" data: no]
Source File: resource:///modules/WebappsInstaller.jsm
This appears to be a Mac-specific issue. Windows also has incorrect behavior, but its a different issue (it overwrites shortcuts, where as Mac stops install).
Need to talk to BD, Jason needs to figure out how many top apps this affects.
These apps are known to risk conflicts (ones known so far, haven't gone through the whole list):
These are possible. The issue probably is going to more heavily prominent on Mac, given that install is blocked if there's a name conflict and since mac has a built-in app store. I'd definitely mark this as a marketplace beta blocker.
Created attachment 618377 [details] [diff] [review]
work in progress, untested
Created attachment 618445 [details] [diff] [review]
This seems to work. Dan, can you test?
This patch should fix both the name collision problem and the icons problem by:
- creating the app bundle in a temporary folder
- checking for an existing app with the same name and adding "(2)" (or higher) to the name
- moving it to the /Applications folder or, if the user has no permissions to do it, to the Desktop
Created attachment 618548 [details] [diff] [review]
- Creates app in temporary folder
- Finds available name in /Applications folder
- moves app to /Application
I'm not using the nsIFile.createUnique function for two reasons:
- It uses Filename-1.app style, and we want Filename (2).app (or to be able to tweak this styling later)
- it would make it harder to do the step in which we create the folder in the tmp dir and then move it to Applications. This is necessary to fix other bugs but I'm doing it here together since I would also need it for this change.
With this change (writing into tmp folder), I don't need to separate the plist file creation done in bug 747601 (as it didn't entirely fix the problem it tried to fix, and the proper way to do it is this one). So I put the .plist creation back to its original function, but I'm still using the getIconForApp callback to do the final move
Comment on attachment 618548 [details] [diff] [review]
>+ * get a unique name for a file on a folder
> /* More helpers for handling the app icon */
> #include WebappsIconHelpers.js
Why do you switch from Filename (2).app to Filename-12.app if the number is >= 10?
I figured if we're not having luck finding an available name up to (9) then changing the filename style made help.