It's too much of a burden for embeddors. Existing users of this method should do one of the following: -Use one of the other methods in nsIPrompt (we may be able to massage their signatures to acccomodate simple cases). -Switch over to using XUL dialogs for more complicated cases (we should make sure that GetInterface on the interface requestor that currently gets you a nsIPrompt also gets you a nsIDOMWindow).

I'll take this one. I need to add a ForgetPassword call at the same time.

i was hoping that all of this password related stuff could be moved out of nsIPrompt.

Here's a patch for this change. I'd like Gagan to verify it first, to make sure I haven't broken http password stuff in the process. Steve should probably look it over too.

wallet and single-signon changes look fine to me.

gagan, the code is in here. can you review it? I'd like to set this to p1. I think all you need to do is review the htpt part and warren will check it in.

PDT reading P2 bugs: Patch looks large -- please review carefully.

I think single-signon features, including prompt interception, should all be in a non-wallet interfac, distinct from, nsIPrompt/nsIDialog. I called it nsIPasswordManager in my patch to 18352.

Why split the PromptPassword/ForgetPassword stuff out of nsIPrompt? What does another interface for this buy us? (We factored the nsIDialog stuff out because embedding thought the DoDialog call was too much to commit to.)

I am going to be adding a AlertCheck() function to nsIPrompt in the next day or so.

PDT thinks P3. Please consider landing after we branch for PR3. (lchiang using trudelle's system/account temporarily)

per email with Jud, changing nsbeta3+ to nsbeta3- on all "embed" keyword bugs since embedding changes will not be made in the mn6 branch. If you feel this bug fix needs to go into mn6 branch, please list the reasons/user impact/risk and nominate for rtm. Thanks.

Netscape has branched, right? Can these land on the trunk, so that I can use them in my reborn work on 18352?

that's the idea. we'd like to get them on the trunk. warren asked me if I could land it but I haven't had a chance to (nor is it looking like I will).

This discussion seems to have stalled. I would like to resume it by proposing that we try to wean ourselves away from nsIPrompt etc. in favor of more specific and complete callback functions. From the point of view of HTTP, for example, it would make much more sense to have a callback function like nsIHTTPEventSink::OnAuthenticate(...), that expects a username and password on return, than to continue using nsIPrompt for this purpose. OnAuthenticate() could also pass other information relevant to authentication such as whether or not the authentication is for a proxy server (see bug 59606 for clarification on why this is needed). I think that getting rid of nsIPrompt would be a big win for embedding... in that well designed event sink objects would provide much finer and more exact control over necko. Moreover, it would have the added benefit of pushing localization issues completely outside of necko. I understand that what I am proposing is a big change, but I think that it is one that we should really consider.

The downside to individual callbacks ifaces is that they add complexity. nsIPrompt is there as a simple/single callback point. we should discuss this at an api review meeting.

True. nsIPrompt is currently good at providing a catch-all type callback mechanism, where new protocols can simply plugin without any modification to the application. I think something like this is still a good idea. But, I think that we should still try to eliminate nsIPrompt in favor of something more specific, yet generic enough to be applicable as a catch-all type callback handler. For example, this catch-all iface could define a GetUsernamePassword method. Then if a protocol specific callback handler is not implemented, this alternate interface could be used. But, like you said... we should discuss this at an api review meeting.

added keyword arch.

Updating QA Contact

-> conrad.

Once the signon specific params are removed from nsIPrompt (see bug 70379), other methods currently in nsIPrompt can be augmented a little to handle the dialogs we currently must do with UniversalDialog.

targeting 0.8.1

-> mozilla0.9

Now that UniversalDialog is not needed for password prompting, its remaining use is for confirmations. The new method here, confirmEx, handles those cases. It also replaces the need for many callers to have localized strings for the same old "Yes", "No", etc. button titles.

So what's used now when you need a textfield and a checkbox?

prompt() has that now.

The diffs replace the remaining uses of universalDialog with a new method: confirmEx. One unfortunate thing: confirmEx has an idl defined constant for which icon to use. This was only to accomadate code in navigatorDD.js. It is the only place which does not use a question icon. Also, due to changes made for bug 72530, in which the way in which the way of specifying the icon for universalDialog was changed, the code in navigatorDD.js icon does not work WRT the icon even before this patch. Point is, does this code need to use a non-standard icon? If not, the iconType param to confirmEx could be done away with.

ppembed and mfcembed - * looks like these two are losing the ability to throw UnivDialog's new equiv (confirmEx()); is that right or am I missing something? any commercial tree usages? I'm inclined to ditch the non-qmark icon case and make it use a std icon. what dialog is it for?

> ppembed and mfcembed - * looks like these two are losing the ability to throw UnivDialog's new equiv It's temporary - I wanted to get the API change in first, then update those. mfcEmbed's impl of UnivDialog was not "Universal" It tried to identify the cases in which Universal dialog was being used for prompt methods. Since the last round of changes to nsIPrompt, UnivDialog will never be used for prompts. So, it's not going to lose any functionality in the meanwhile. >any commercial tree usages? fortunately, no > I'm inclined to ditch the non-qmark icon case and make it use a std icon. what > dialog is it for? It's for the dialog which comes up when you drag a URL shortcut from the desktop onto the homepage icon in the tool bar. It asks something like: "Do you want to make this your homepage?" The confirm dialog is supposed to have an icon of the home page. Currently, it doesn't work anyway. I'm also inclined to have it use the std question mark icon.

r=morse for changes to nsPermissions.cpp and wallet.cpp But you should also remove Yes and No from wallet.properties and cookie.properties, both in extensions/wallet/src.

Steve, what about the ones in extensions/wallet/src/resources/locale... ?

That's directory is not mine. I never created it nor authorized it's creation. And it's not part of the build.

Correction: Changing QA contact for the Embed API bugs to David Epstein.

Simon, can you sr - particularly the .js code, much of which is in editor?

What does "confirmEx" mean? That's a nasty method name. I'd also suggest using AStrings in the prompt interface(s), and for things like GetLocaleString(). Do you really need AutoStringFree? Doesn't nsXPIDLString do the same job? Otherwise, I applaud the changes.

Ex is ugly, but it's for Extended. We already have confirm, and confirmCheck, so for the more elaborate confirm method, I couldn't think of a better name. Another point - maybe 3 versions of confirm are two much. There aren't many callers of confirmCheck and that can easily be done using confirmEx and the constant for STD_OK_CANCEL_BUTTONS. As far as the string changes, I'll see if AutoStringFree can be replaced with nsXPIDLString. Usings AStrings in the prompt interface(s)? That might be massive work - and how is this done in idl?

I grepped (instead of lxr.netscape.com) my commercial tree pull for usage cases (as a double check); found none. commercial is clear.

AString in idl -> nsAReadableString/nsAWritableString. They allow you to avoid extra allocations and copies, and are totally transparent to JS users.

advancedConfirm? confirmWithOptoins? confirmExtended, even?

just spoke w/ scc on the phone about the new string stuff which simon mentioned. he enumerated the pros (see the string docs in mozilla/string/docs for working revisions of docs) of using these classes. I'm going to make the call here not to use them in this iface. The public DOM interfaces use them, so at that level embeddors will indeed be exposed to them, but we're going to forgoe their usage in this case. When the string namespace is frozen, we can talk about proliferating the new string stuff through all the embedding ifaces, but until then, and because these ifaces are called very, relatively, infrequently (thus no real performance gain from the new string impls would be realized), we're going to skip them here. Our internal impls can obviously use nsAString for the utility gain, but we'll use raw pointers here. We need to hash out our public string story.

Simon, the "Ex" suffix for Extended is at least standard on Windows. I've seen it a few times in our code base as well. The alternatives to it aren't that much better, so I'll stick with it for now. Anyway the point of this is an API change and the riddance of UniversalDialog. If we think of a better name later, a simple name change can be done then. You're right about the AutoStringFree class. nsXPIDLString does the job just as well. I was moving this code from one place to the other - AutoStringFree came along with it. I will replace it with nsXPIDLString and reduce the code size a little :-)

sr=rpotts

r=valeski

sr=sfraser

Fix checked in. UniversalDialog is gone.

How do embedding apps get notified of cookies now that UniversalDialog is no more?

All the call sites in cookie were changed to use confirmEx for that purpose.

Wait, why are embedded apps using the prompt interface for cookie notification? Isn't that why we have a cookie manager interface? Like, isn't that the _whole_point_?

> Wait, why are embedded apps using the prompt interface for cookie notification? Back up, I don't think he said that.

He said: > How do embedding apps get notified of cookies now that UniversalDialog is no > more? which certainly sounds like embedding apps got notified of cookies with UniversalDialog before. Doesn't it?

reassigned QA contact to dsirnapalli. He's testing this interface.

-- UniversalDialog method in nsIPrompt no more exists. changing to verified.