The intent of the change in bug 758696 was to allow debugger server embedders to supply their own user-acceptance handler. However, the way I implemented it requires *all* embedders to do so, even add-ons, like NetMonitor and the Profiler. It would be much easier to use the remote debugging protocol in other tools if the server specified its own default user-acceptance handler, while allowing it to be overridden by embedders at the same time. In the same patch I'll fix the user-acceptance check to only occur on socket connections, not nsIPipe ones.

This appears to work fine and doesn't regress b2g desktop. I'll try fennec as well. Rob, I copied the localization file under a new devtools directory, since that's what we do in browser/ as well. Mark, since this is security sensitive code you may want to take a look, although there should be no user perceivable change. Axel, do I need to do anything special when moving strings from browser/ to toolkit/? There are no identifier or content changes, just a location change.

Fennec is unaffected as well.

Comment on attachment 650552 [details] [diff] [review] Patch v1 Review of attachment 650552 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for asking, this looks good. Once this lands, it'd be great if you could do a quick post to mozilla.dev.l10n giving people a heads up that already translated these strings.

Thanks Axel, I'll make a note to post to mozilla.dev.l10n.

Comment on attachment 650552 [details] [diff] [review] Patch v1 +/** + * Localization convenience methods. + */ +let L10N = { + + /** + * L10N shortcut function. + * + * @param string aName + * @return string + */ + getStr: function L10N_getStr(aName) { + return this.stringBundle.GetStringFromName(aName); + } +}; + you could just make this a straight-up function, but if you like having the object, you can keep it. Ok, I think this change makes sense. Allowing actors to specify their own connection callback seems to make sense rather than foisting one onto them.

(In reply to Rob Campbell [:rc] (:robcee) from comment #5) > +/** > + * Localization convenience methods. > + */ > +let L10N = { > + > + /** > + * L10N shortcut function. > + * > + * @param string aName > + * @return string > + */ > + getStr: function L10N_getStr(aName) { > + return this.stringBundle.GetStringFromName(aName); > + } > +}; > + > > you could just make this a straight-up function, but if you like having the > object, you can keep it. I suspect that we may need to add a getFormatStr at some point in the future, like we do in debugger-controller.js: http://mxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/debugger-controller.js#1615

Comment on attachment 650552 [details] [diff] [review] Patch v1 Review of attachment 650552 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me.

Updated the patch against current m-c.