We need IC support for getting protocol handlers, for two reasons: 1. Need to get handler for unknown protocols (e.g. afp). 2. Need to be able to have embedding apps control what handles different protocols.

*** Bug 68513 has been marked as a duplicate of this bug. ***

I think we should still use the prefs API for this. But, the prefs front end on the Mac should offer a way to sync these settings with those of IC. The prefs here have two levels: our app (or the embedding app), and system wide. I don't think we should blur this distinction as IE does. What would be good is this: In the prefs UI for Mac, add these buttons: (1) "Use System Prefs" - Reads all helper app info from IC and imports it into our prefs. (2) "Set As System Prefs" - Writes our current helper app prefs to IC making it global These buttons could act on the whole list. Or, we could make the buttons act on only the current selection in the helper apps prefs list.

Eeek. Writing prefs to IC like this is fraught with user experience problems. When you write to IC, you open up a bunch of UI problems. Basically, in this mode, there are now two places in which the user can edit Helper App prefs -- in the IC control panel, and in Mozilla's prefs. Edit in one, and you're never sure htat the other one won't clobber your changes. And what if Mozilla is running, and you edit the IC prefs? It becomes very messy, unless you remove the UI in Mozilla, and force the user to go to the Internet control panel to make changes. For some enlightening discussion on this issue, look for old posts on the .mac newsgroup.

> Edit in one, and you're never sure htat the other one won't clobber your > changes. No way - or I mis-stated what I meant. I said that we should be able to "sync" (manually) between system prefs and our own. There's no clobbering or auto-syncing going on. There shouldn't be any question.

I simply cannot use nor recommend Netscape use on Macintosh systems without Netscape having the ability to recognize and use protocol handlers defined in Internet Config. Please add support for IC on Mac OS.

Yep, I agree. This is a must-have feature for MacOS browsers...

Adding bunch o' keywords. This should be fixed, and in fact, I have just that. The only thing I'm thinking of adding is an alert when we look for the external protocol handler and it's mozilla/netscape. Right now, if that's the case, mozilla just fails silently.

Just attached my patches so that we won't lose them (I think I already lost one of my USB ports on my PowerBook, so better safe than sorry). The only thing left is to alert the user when mozilla/netscape is registered in IC as a handler to an external protocol that we're trying to launch. I was thinking of putting up an alert inside nsInternetConfigService::HasProtocolHandler, but that would looks kinda funny having a native mac alert dialog in mozilla/NS6, not that mac users care. Also, for embedding clients, it's probably better to return a specific error and have the clients (along with mozilla) figure out how to alert the user.

> I was thinking of putting up an alert inside > nsInternetConfigService::HasProtocolHandler, but that would looks kinda funny > having a native mac alert dialog in mozilla/NS6 Why can't you use nsIPromptService? That is the only way to go for both embedding and seamonkey. For seamonkey, it will use XUL and for embedding, if the embeddor has overridden the prompt service component, it will use a native dialog.

Three new patches to put up dialog. 1) nsInternetConfigService::HasProtocalHandler() returns NS_ERROR_NOT_AVAILABLE if the creator type of the protocol handler is the same as our current running app creator type. 2) check for NS_ERROR_NOT_AVAILABLE in nsOSHelperAppService::ExternalProtocolHandlerExists(), and put up a dialog that says we can't handle the protocol. I would be more than happy for someone to suggest ways to clean up this code that puts up the dialog. 3) add localizable string in helperAppLauncher.properties used in dialog At this point, I consider this bug fixed. Looking for r= and sr=

nsInternetConfigService::HasProtocolHandler() should init *_retval to PR_FALSE. + if ( err ) + return NS_ERROR_FAILURE; + else + { Prefer if (err != noErr). Why the 'else'? + NS_ConvertASCIItoUCS2 brandShortName("brandShortName"); + rv = brandBundle->GetStringFromName(brandShortName.get(), getter_Copies(brandName)); Try using NS_LITERAL_STRING("brandShortName") here and in the other places with this pattern. This code also needs to be written without gotos, since you are jumping over nsCOMPtrs, and may run into crashes because of the destruction of uninitialized objects. + if (aURL) + aURL->GetSpec(&url); + nsCOMPtr<nsIInternetConfigService> icService (do_GetService(NS_INTERNETCONFIGSERVICE_CONTRACTID)); + if (icService) + rv = icService->LaunchURL(url); This code leaks the url char*.

Yeah, I was debating about the goto's but wanted to try something new instead of hugely nested if statements, but I forgot about those nsCOMPtr's in my haste to code this stuff up. As for this: + if ( err ) + return NS_ERROR_FAILURE; + else + { No fucking clue what I was thinking with that else clause. ;-)

paul says its almost done!

Latest patch to nsInternetConfigService.cpp removes that silly else clause. Latest patch to nsOSHelperAppService.cpp does away with the goto, gets rid of all the NS_ConvertASCIItoUCS2 and replaces them with NS_LITERAL_STRING, check for null from GetSpec(), call nsMemory::Free on url returned from GetSpec().

This code scares me: + Str255 pref = kICHelper; + nsCRT::memcpy(pref + pref[0] + 1, protocol, nsCRT::strlen(protocol)); + pref[0] = pref[0] + nsCRT::strlen(protocol); You have no protection against running off the end of the Str255 with a long protocol name. Sorry to only whine now about this, but the string building foo at: + nsAutoString str(brandName.get()); + str.AppendWithConversion(' '); + str.Append(errorStr.get()); + str.AppendWithConversion(' '); + str.AppendWithConversion('\"'); + str.AppendWithConversion(aProtocolScheme, nsCRT::strlen(aProtocolScheme)); + str.AppendWithConversion('\"'); seems very un-localizable (word order might differ). Don't we have a sprintf-type way of doing i18n-friendly string substitutions?

Latest nsOSHelperAppService.cpp patch changes '\"' to '"'. Hmmm, what was I thinking.

Simon, how about if I change the alert string in the properties file to be something like '%1 cannot handle the protocol "%2"' and then do a ReplaceSubstring() fro %1 and %2?

You should use the 'formatStringFrom..' methods on nsIStringBundle.

Done and done. Ok, Simon. What else do I need to fix. I'm actually enjoying this because I'm learning tons. Sick isn't it? ;-) Actually, what's really sick is that you have to learn these things essentially through word of mouth.

Looks good now. sr=sfraser

r=alecf

Fix checked in

V: CFM is gone anyhow. If IC lives on in Mac OS X in some strange way I am not aware of, please let me know.

The IC APIs live on in OS X. Apple sez you should use LaunchServices instead but until they actually provide all the functionality of IC in LS we're sorta stuck using IC.

Steve: so, can you give me more info so I can understand how to write a couple test cases?