748199 - Installation fails if name of app conflicts with native applications

Status: VERIFIED FIXED

(Firefox Graveyard :: Web Apps, defect)

Description

[Christopher Van Wiemeersch [:cvan]]

Attachment: App installation not allowed

User Agent:
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)

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

Comment 1

[Jason Smith [:jsmith]]

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.

Comment 2

[Jason Smith [:jsmith]]

Error Console:

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
Line: 36

Comment 3

[Jason Smith [:jsmith]]

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

Comment 4

[Jason Smith [:jsmith]]

Need to talk to BD, Jason needs to figure out how many top apps this affects.

Comment 5

[Jason Smith [:jsmith]]

These apps are known to risk conflicts (ones known so far, haven't gone through the whole list):

	* Springpad
	* Timer
	* Checkers
	* Sandglaz
	* Chess
	* Calculator
	* Soundcloud
	* Kicksend

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.

Comment 6

[:Felipe Gomes (needinfo for replies!)]

Attachment: WIP

work in progress, untested

Comment 7

[:Felipe Gomes (needinfo for replies!)]

Attachment: Patch

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

Comment 8

[:Felipe Gomes (needinfo for replies!)]

Attachment: Patch v2

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

[Markus Stange [:mstange]]

Comment on attachment 618548 [details] [diff] [review]
Patch v2

>+/**
>+ *  get a unique name for a file on a folder
>+ */
> /* More helpers for handling the app icon */
> #include WebappsIconHelpers.js

stray comment

Why do you switch from Filename (2).app to Filename-12.app if the number is >= 10?

Comment 10

[:Felipe Gomes (needinfo for replies!)]

I figured if we're not having luck finding an available name up to (9) then changing the filename style made help. 

https://hg.mozilla.org/mozilla-central/rev/8180e2442cd2