unfortunately, I missed this in my review of bug 252189... anyway, that bug added a way to get a friendly app description; GetApplicationDescription should be able to use that.

Comment on attachment 190271 [details] [diff] [review] patch >Index: uriloader/exthandler/win/nsOSHelperAppService.cpp >+ nsAutoString typeName(utf16Scheme); Why do you need this? GetDefaultAppInfo should take a const nsAString as its first arg. >+ GetDefaultAppInfo(typeName, _retval, getter_AddRefs(app)); Isn't |typeName| supposed to be something like "pngfile" or whatever? Does passing something like "mms" here really do the right thing? I guess this is looking for the command under the same registry key as GetDefaultAppInfo, so it must be... sr=bzbarsky with the extra nsAutoString nixed.

> GetDefaultAppInfo should take a const nsAString as its first arg. it doesn't... hmm, but yeah, it should. I'll make a new patch.

Comment on attachment 190271 [details] [diff] [review] patch >Index: uriloader/exthandler/win/nsOSHelperAppService.cpp >+ const NS_ConvertUTF8toUTF16 utf16Scheme(aScheme); A URI scheme is restricted to ASCII, so you should be able to use NS_ConvertASCIItoUTF16 without any loss what-so-ever. >+ nsCOMPtr<nsIFile> app; >+ nsAutoString typeName(utf16Scheme); >+ GetDefaultAppInfo(typeName, _retval, getter_AddRefs(app)); Why is the first parameter to GetDefaultAppInfo non-const? The implementation in this file certainly does not seem to modify the string. Can you change it to be const and so avoid having to copy utf16Scheme? >+ nsAutoString cmdString(utf16Scheme); If you must copy utf16Scheme, then it seems like you should only have to do so once. You could avoid the third copy with code like this: nsAutoString &cmdString = utf16Scheme; But, I'd recommend avoiding all copies if possible.

Can I append to an NS_ConvertASCIItoUTF16 string?

> Can I append to an NS_ConvertASCIItoUTF16 string? Yes, it is just a subclass of nsAutoString :-)

Comment on attachment 192192 [details] [diff] [review] don't copy >Index: uriloader/exthandler/win/nsOSHelperAppService.cpp >+ NS_ConvertASCIItoUTF16 utf16Scheme(aScheme); >+ >+ nsCOMPtr<nsIFile> app; >+ GetDefaultAppInfo(utf16Scheme, _retval, getter_AddRefs(app)); >+ >+ if (!_retval.Equals(utf16Scheme)) >+ return NS_OK; >+ >+ // Fall back to full path >+ utf16Scheme.AppendLiteral("\\shell\\open\\command"); > nsresult rv = regKey->Open(nsIWindowsRegKey::ROOT_KEY_CLASSES_ROOT, >+ utf16Scheme, > nsIWindowsRegKey::ACCESS_QUERY_VALUE); In situations such as this, I'd be tempted to give the string object a generic name such as "buf" since it is not always used to represent a protocol scheme. r=darin

Comment on attachment 192192 [details] [diff] [review] don't copy I'll rename the variable when checking in This patch changes the application name in the dialog that appears when launching an external application for a protocol to be a nice name instead of just the filename. should be low risk; certainly limited to to that dialog.

trunk: Checking in uriloader/exthandler/win/nsOSHelperAppService.cpp; /cvsroot/mozilla/uriloader/exthandler/win/nsOSHelperAppService.cpp,v <-- nsOSHelperAppService.cpp new revision: 1.64; previous revision: 1.63 done Checking in uriloader/exthandler/win/nsOSHelperAppService.h; /cvsroot/mozilla/uriloader/exthandler/win/nsOSHelperAppService.h,v <-- nsOSHelperAppService.h new revision: 1.25; previous revision: 1.24 done MOZILLA_1_8_BRANCH: Checking in uriloader/exthandler/win/nsOSHelperAppService.cpp; /cvsroot/mozilla/uriloader/exthandler/win/nsOSHelperAppService.cpp,v <-- nsOSHelperAppService.cpp new revision: 1.63.4.1; previous revision: 1.63 done Checking in uriloader/exthandler/win/nsOSHelperAppService.h; /cvsroot/mozilla/uriloader/exthandler/win/nsOSHelperAppService.h,v <-- nsOSHelperAppService.h new revision: 1.24.4.1; previous revision: 1.24 done