393122 - external helper app service / handler info simplification tracking bug

Status: NEW

(Firefox :: File Handling, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

During the various web-based protocol handling work that we've been doing, a bunch of possible ways to simplify the external helper app service and handler info have suggested themselves.  Rather than filing all the possible bugs now, I'll list some of changes I suspect we'll want here, and we can spin off individual bugs as we get to them and make them block this one.

Some (most?) of these changes are likely to break ports when they happen, but they should collectively make this code less fragile and easier to maintain.

A) split out nsLocalHandlerApp to have a default implementation which can be overridden by an OS-specific one.

B) have each nsIHandlerApp have its own implementation of LaunchWithURI and LaunchWithFile.

C) make the OS default handler be its own nsILocalHandlerApp, possibly just a subclass of the OS-specific nsLocalHandlerApp.

Doing these things allows LaunchWithURI and LaunchWithFile on the nsI{MIME,Handler}Info to simply call through to the appropriate nsIHandlerApp rather than sending things through a bunch of somewhat complex logic.

Further down the road, some other things that would be good to do would be:

D) right now, nsI{MIME,Handler}Info is initialized from OS-specific data (if any exists), and then passed up to the helper app service that fills in any data persisted in RDF.  This is object model makes it somewhat difficult wrap one's head around how it all fits together.  I think the OS-specific data wants to simply be a separate object implementing something like nsIOSHandlerInfo which contains any and all data that comes from and/or is persisted by the OS itself.  This could then be an attribute on nsI{MIME,Handler}Info.

E) Make all reading and writing of the RDF data persistence happen through nsIHandlerService.  This includes stuff that's currently done manually in the external helper app service and the unknown content-type handler dialog.  At some point, it's possible that all of this persistence stuff wants to really happen in the front-end code, and none in the external helper app service; I'm not entirely sure.

F) we may also want to split up the nsIMIMEInfo and the protocol handlers, but some of these other refactorings may make that less worth doing.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

For the Applications prefpane being implemented over in bug 377784, it would be useful to be able to distinguish between nsIHandlerInfo objects that represent user configuration and those that represent default settings from the OS.

It's not a requirement; the patch I'm working on for that bug works around the limitation; but it would be useful to have.  I think for that we'd need to implement item D.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Another cleanup, this one simple: move nsIMIMEInfo::equals to nsIHandlerInfo::equals and make the implementation in nsMIMEInfoImpl.cpp not be specific to nsIMIMEInfos.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

This is more blue sky, and I'm not sure how appropriate it is for this bug, but it would be great if nsIMIMEInfo contained an attribute for the plugin that handles the type (if any), and, if the plugin is enabled, set preferredAction to a new nsIHandlerInfo::usePlugin nsHandlerInfoAction value.

Currently, the preferences interface to handlers (which is being overhauled in bug 3777784) loads that information separately from navigator.plugins and then composes a JavaScript object that contains both the handler info object and the plugin information.  If the handler info object contained that information, then the interface could simply use the handler info object directly.

Another blue sky request would be for the feed handling configuration, which is currently stored in preferences, to be stored in the handlers datastore along with other handlers, as the preferences interface will have to separately load that too and then synthesize data structures to represent it.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Now that we've made nsIWebHandlerApp inherit from nsIHandlerApp, we should also try to make nsIWebContentHandlerInfo inherit from or possibly be replaced by nsIWebHandlerApp.

Comment 5

[Shawn Wilsher :sdwilsh]

(In reply to comment #4)
> Now that we've made nsIWebHandlerApp inherit from nsIHandlerApp, we should also
> try to make nsIWebContentHandlerInfo inherit from or possibly be replaced by
> nsIWebHandlerApp.
I totally agree (and was thinking that when I hooked up the stuff for web handler registration in the first place).  I'd be happy to take that once I'm in school.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #5)
> (In reply to comment #4)
> > Now that we've made nsIWebHandlerApp inherit from nsIHandlerApp, we should also
> > try to make nsIWebContentHandlerInfo inherit from or possibly be replaced by
> > nsIWebHandlerApp.
> I totally agree (and was thinking that when I hooked up the stuff for web
> handler registration in the first place).  I'd be happy to take that once I'm
> in school.

I filed this one as bug 394710.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: split out nsLocalHandlerApp to be per OS, and move LaunchWithURI, p1 (newer revs in bug 394483)

This is a work in progress; still needs a little love.

Comment 8

[James Storro]

Attachment: same problem

Thanks from vesmarazum1975.