The idea is to reduce code duplication here. We should get a JS module that has an implementation for nsIWebHandlerApp and nsILocalHandlerApp.

requesting blocking because this blocks a blocking bug

it looks like it works for local ones, not web ones

bye bye JS :(

Comment on attachment 275685 [details] [diff] [review] v2.0 sr=dmose

Comment on attachment 275685 [details] [diff] [review] v2.0 ack....

third time is the charm right?

Comment on attachment 275711 [details] [diff] [review] v3.0 Nice work; this makes the code much cleaner. A few tweaks would be good: * name the class nsLocalHandlerAppImpl so that it lines up with the mimeinfo class naming * move the existing file to nsLocalHandlerAppImpl.{h,cpp} for clarity (just doing "cvs remove" and "cvs add" should be fine; I don't think the blame history here is substantial enough to be worth preserving). * name the values in the setters something more descriptive than aVal. * does the JS component need to be added to any packaging files, or is that done automagically by the build system? sr=dmose

sr=dmose Addresses comments I'll get the seamonkey and tbird packages files done at checkin

Comment on attachment 276041 [details] [diff] [review] v3.1 >+function nsWebHandlerApp() >+{ >+} >+nsWebHandlerApp.prototype = >+{ >+ get name() >+ { >+ return this._name; >+ }, Nit: per style elsewhere (although this varies), and as demonstrated in examples in the JavaScript style guide <http://developer.mozilla.org/en/docs/JavaScript_style_guide>, it would be better to place opening braces next to the block identifier rather than on the next line, and, for empty blocks, to place the closing brace next to the opening one, i.e.: function nsWebHandlerApp() {} nsWebHandlerApp.prototype = { get name() { return this._name; },

really? wow, that really varies throughout the codebase then, doesn't it? I'll see if biesi has a preference, but in the past, this is what my reviewers have had me done (or haven't said anything about it)

(In reply to comment #10) > Nit: per style elsewhere (although this varies), and as demonstrated in > examples in the JavaScript style guide > <http://developer.mozilla.org/en/docs/JavaScript_style_guide>, it would be > better to place opening braces next to the block identifier rather than on the > next line, and, for empty blocks, to place the closing brace next to the > opening one, i.e.: I agree.

Comment on attachment 276041 [details] [diff] [review] v3.1 nsLocalHandlerAppImpl.cpp I don't like the "Impl" in filenames very much, it's really superfluous (what else would be in a cpp file?) So I'd rename this to nsLocalHandlerApp.cpp

for checkin

for checkin. yay for buildbot try patch finding a missing file in the patch!

Checking in netwerk/mime/public/nsIMIMEInfo.idl; new revision: 1.34; previous revision: 1.33 Checking in docshell/build/nsDocShellModule.cpp; new revision: 1.30; previous revision: 1.29 Checking in uriloader/exthandler/Makefile.in; new revision: 1.68; previous revision: 1.67 Checking in uriloader/exthandler/nsCExternalHandlerService.idl; new revision: 1.8; previous revision: 1.7 Checking in uriloader/exthandler/nsExternalHelperAppService.cpp; new revision: 1.337; previous revision: 1.336 Removing uriloader/exthandler/nsHandlerAppImpl.cpp; new revision: delete; previous revision: 1.4 Removing uriloader/exthandler/nsHandlerAppImpl.h; new revision: delete; previous revision: 1.6 Checking in uriloader/exthandler/nsLocalHandlerApp.cpp; initial revision: 1.1 Checking in uriloader/exthandler/nsLocalHandlerApp.h; initial revision: 1.1 Checking in uriloader/exthandler/nsWebHandlerApp.js; initial revision: 1.1 Checking in uriloader/exthandler/tests/unit/test_handlerService.js; new revision: 1.9; previous revision: 1.8 Checking in browser/installer/unix/packages-static; new revision: 1.126; previous revision: 1.125 Checking in browser/installer/windows/packages-static; new revision: 1.136; previous revision: 1.135 Checking in suite/installer/windows/packages; new revision: 1.30; previous revision: 1.29 Checking in mail/installer/windows/packages-static; new revision: 1.70; previous revision: 1.69