I noticed this today. We don't return from this method anymore so this likely always fails. http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/WebappRT.js#72

(In reply to Wesley Johnston (:wesj) from comment #0) > I noticed this today. We don't return from this method anymore so this > likely always fails. > > http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/ > WebappRT.js#72 It sure does! Remote debugging is also broken by the server-side bug 988644, which ozten is working on. But once he fixes that, we'll still need to fix this in order to get it working again. I have a work in progress; I'll attach it shortly.

Since EventDispatcher.sendResponse requires a JSONObject, we might as well return an actual boolean. (Aside: I wish sendMessageToJava pre-parsed the response before passing it to the callback.)

Comment on attachment 8399563 [details] [diff] [review] patch v2: be compatible with changes in bug 989098 Review of attachment 8399563 [details] [diff] [review]: ----------------------------------------------------------------- I don't have a strong opinion on it, but if you want to simplify, you can avoid the extra JSONObject here. ::: mobile/android/base/GeckoApp.java @@ +721,1 @@ > EventDispatcher.sendResponse(message, ret); You can actually just pass this boolean straight to the sendResponse function now. ::: mobile/android/chrome/content/WebappRT.js @@ +66,5 @@ > } > > #ifdef MOZ_ANDROID_SYNTHAPKS > // If the app is in debug mode, configure and enable the remote debugger. > + sendMessageToJava({ type: "NativeApp:IsDebuggable" }, (response) => { Then here response would just be the bool "isDebuggable".

(In reply to Wesley Johnston (:wesj) from comment #4) > I don't have a strong opinion on it, but if you want to simplify, you can > avoid the extra JSONObject here. I do have a strong opinion, which is that I want to simplify! But I also want to uplift to Fx30, since regressing bug 946344 landed there. And bug 989098, which added the ability to pass a boolean to sendResponse, landed in Fx31. So I'll leave this the way it is for now in the interest of simplifying the uplift request.

Comment on attachment 8399563 [details] [diff] [review] patch v2: be compatible with changes in bug 989098 [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 946344. User impact if declined: Webapp developers won't be able to debug their apps. Testing completed (on m-c, etc.): The fix landed on Central earlier today. Risk to taking this patch (and alternatives if risky): It's a small, low-risk change. String or IDL/UUID changes made by this patch: None.