Closed Bug 49758 Opened 24 years ago Closed 23 years ago

Need to get protocol handlers from Internet Config

Categories

(Core :: Networking, defect, P3)

PowerPC
Mac System 8.5
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: sfraser_bugs, Assigned: paulkchen)

References

Details

Attachments

(12 files)

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.
Keywords: nsmac2
Status: NEW → ASSIGNED
*** 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.
Keywords: 4xp, nsbeta1, nsCatFood
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.
Blocks: 5721
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!
Keywords: nsbeta1nsbeta1+
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
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
QA Contact: tever → benc
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.
Status: RESOLVED → VERIFIED
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?
You need to log in before you can comment on or make changes to this bug.