223310 - nsIKeygenThread API isn't embedding friendly

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect)

Description

[Crispin Flowerday (not receiving bug mail)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6a) Gecko/20031015 Galeon/1.3.9
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6a) Gecko/20031015 Galeon/1.3.9

The nsIKeygenThread API reqires an nsIDOMWindowInternal object, on which it just
calls the Close function.

This isn't very embedding friendly, for instance in galeon we have had to create
an entire dummy nsIDOMWindowInternal implementation of which just the Close()
function is implemented.

Creating a separate callback API would make altering the dialog for other
applications a lot simpler.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Crispin Flowerday (not receiving bug mail)]

I should point out, this is a problem when creating a custom implementation of
the nsIGeneratingKeypairInfoDialogs class.

Comment 2

[John G. Myers]

Mass reassign ssaux bugs to nobody

Comment 3

[John G. Myers]

I don't have the bandwidth to do this, but would be willing to review patches.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

-> me, galeon doesn't like this

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

this functionality is used at "generate certificate" on
http://patches.theflowerdays.com/ca/

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 165845 [details] [diff] [review]
patch

oops, forgot to change the uuid of the interface

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

Comment 9

[Darin Fisher]

Comment on attachment 165896 [details] [diff] [review]
patch v2

>Index: security/manager/pki/resources/content/createCertInfo.js

>+  var obs = {
>+    observe : function keygenListenerObserve(subject, topic, data) {
>+      if (topic == "keygen-finished")
>+        window.close();
>+    }
>+  };

Doesn't this need a QueryInterface implementation?  Won't it trigger
Assert_NoQueryNeeded (from nsCOMPtr.h) in debug builds?


>Index: security/manager/ssl/public/nsIGenKeypairInfoDlg.idl

>+ * INTERFACES THAT NEED TO BE IMPLEMENTED:
>+ *   nsISupports
>+ *   nsIGeneratingKeypairInfoDialogs

nsISupports is implied by nsIGeneratingKeypairInfoDialogs, so maybe
there is no need to mention it here?


> #define NS_GENERATINGKEYPAIRINFODIALOGS_CONTRACTID "@mozilla.org/nsGeneratingKeypairInfoDialogs;1"

Consider adding a linebreak to make this readable on narrow (80 char)
displays.


sr=darin

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

if http://www.mozilla.org/scriptable/faq.html#i12 (paragraph 8) is to be
trusted, then I don't need an explicit qi impl, because xpconnect will make one
up for me

Comment 11

[Darin Fisher]

Ah, very cool!  I didn't realize that xpconnect could do that :-) 

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: final patch

also changes nsIDownload.idl to not list nsISupports. for that file, I'm still
leaving nsITransfer in - I think it's a good thing to list all interfaces that
need to be listed in NS_IMPL_ISUPPORTSx here, i.e. all things you can QI to.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in for mozilla 1.8a6.