Closed Bug 223310 Opened 19 years ago Closed 18 years ago

nsIKeygenThread API isn't embedding friendly

Categories

(Core Graveyard :: Security: UI, defect)

Other Branch
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: crispin, Assigned: Biesinger)

References

()

Details

Attachments

(1 file, 2 obsolete files)

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:
Status: UNCONFIRMED → NEW
Ever confirmed: true
I should point out, this is a problem when creating a custom implementation of
the nsIGeneratingKeypairInfoDialogs class.
Mass reassign ssaux bugs to nobody
Assignee: ssaux → nobody
Component: Daemon → Client Library
I don't have the bandwidth to do this, but would be willing to review patches.
-> me, galeon doesn't like this
Assignee: nobody → cbiesinger
OS: Linux → All
Hardware: PC → All
Attached patch patch (obsolete) — Splinter Review
Attachment #165845 - Flags: review?(jgmyers)
Comment on attachment 165845 [details] [diff] [review]
patch

oops, forgot to change the uuid of the interface
Attachment #165845 - Flags: review?(jgmyers) → review-
Attached patch patch v2 (obsolete) — Splinter Review
Attachment #165845 - Attachment is obsolete: true
Attachment #165896 - Flags: review?(jgmyers)
Status: NEW → ASSIGNED
Attachment #165896 - Flags: review?(jgmyers) → review+
Attachment #165896 - Flags: superreview?(darin)
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
Attachment #165896 - Flags: superreview?(darin) → superreview+
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
Ah, very cool!  I didn't realize that xpconnect could do that :-) 

Attached patch final patchSplinter Review
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.
Attachment #165896 - Attachment is obsolete: true
checked in for mozilla 1.8a6.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Product: PSM → Core
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.