Created attachment 380618 [details] [diff] [review] Pass a checkState even if we don't use it There are probably other places where we are calling confirmEx and other nsIPromptService methods passing nsnull to out parameters, I can write a patch to fix those too if this approach is fine. I couldn't find anything in the xpconnect documentation about passing null as out parameter, but looking at the code it seems like they are refused intentionally. Suggestions about better ways of fixing the issue are welcome.
Attachment #380618 - Flags: review?
Here is where the call is aborted because of the null parameter: http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/src/xpcwrappedjsclass.cpp#1463
Comment on attachment 380618 [details] [diff] [review] Pass a checkState even if we don't use it You really want to request review from a particular person, so the review request doesn't get lost... If you're not sure whom to ask, check in #developers on irc.mozilla.org. ;) That said, this looks great; passing null for an inout param is in fact an API violation... If you do want to look for other instances of such calls to ConfirmEx, that would be great; once we eliminate them, I think we should remove the null-check in ConfirmEx and replace it with either a fail-on-null or just an assertion. That should be a separate bug, though. Do you need this landed on the 1.9.1 branch as well as trunk? I'll land on trunk as soon as we reopen it (hopefully a few days at most).
Attachment #380618 - Flags: review? → review+
(In reply to comment #3) > (From update of attachment 380618 [details] [diff] [review]) > Do you need this landed on the 1.9.1 branch as well as trunk? I'll land on > trunk as soon as we reopen it (hopefully a few days at most). Fennec has plans to reimplement nsIPromptService in JS for it's 1.0 release. That won't be 1.9.1, but would be 1.9.1.x I'm not sure it wanted-1.9.1 is good enough for that.
tracking-fennec: --- → ?
Created attachment 381322 [details] [diff] [review] Always pass a checkState to nsIPromptService methods even if we don't use it
The updated patch should fix all the nsIPromptService calls, I hope I've not missed any. Sorry about the missing review person. I thought I had typed on in, maybe I typoed it or something :)
Landing in 1.9.1 would be good if possible.
Assignee: nobody → mpgritti
Pushed http://hg.mozilla.org/mozilla-central/rev/7fd31bcba0fc Nominated for 1.9.1.x. Thanks for the patch!
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Not blocking 184.108.40.206, but we'll consider a patch for 220.127.116.11 or later.
You need to log in before you can comment on or make changes to this bug.