173844 - Cookie confirmation dialog should be a child of originating window

Status: RESOLVED WORKSFORME

(Core :: Networking: Cookies, defect, P3)

Description

[Koike Kazuhiko]

If two or more navigator windows are opened, cookie confirmation
dialog for non-active window opens as a child of active window.

Steps to reproduce:
(1) Check "Ask me before storing a cookie" in Preferences dialog.
(2) Open two windows.
(3) Access to a webpage which send cookies(ex. buglist.cgi of Bugzilla)
 and activate another window.


bug 128554 may be related. I don't know why cookie confirmation dialog
is modal now.

Comment 1

[Darin Fisher]

*** Bug 170172 has been marked as a duplicate of this bug. ***

Comment 2

[Darin Fisher]

*** Bug 188566 has been marked as a duplicate of this bug. ***

Comment 3

[Darin Fisher]

-> me

Comment 4

[Mike Pinkerton (not reading bugmail)]

*** Bug 199985 has been marked as a duplicate of this bug. ***

Comment 5

[Mike Pinkerton (not reading bugmail)]

this is really bad for macosX, and any macos-native embedding app (camino,
cocoaEmbed). modal dialogs are just wrong and this is a regression from the
nsIPrompt api we had on the 1.0 branch.

requesting TM other than future, we need to address this in the 1.4 timeframe.

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

From Bug 199985:
> Yes, I think so. Here,
>
http://lxr.mozilla.org/seamonkey/source/extensions/cookie/nsCookiePermission.cpp#75,
> the method calling CookieDialog is being passed a parent window. Why isn't it
> being used?

The parent not being used is a bug. But that function is called from
thttp://lxr.mozilla.org/seamonkey/source/extensions/cookie/nsCookies.cpp#1773.
There, only a prompt service is known. It is not possible (afaik) to get a
parent from the prompt service.

Comment 7

[Darin Fisher]

pink: not a trivial bug by any means, or we would have fixed it by now.  going
to be a challenge to get this fixed by 1.4.  if anyone is interested in helping,
please let me know, and i'll explain what i think needs to be done.

Comment 8

[Charles Rosendahl]

plussed per edt meeting

Comment 9

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: first shot

This adds a getter for the parent to nsIPrompt.idl. So that makes it possible
to grab the right parent window out of it.
I might still fail when nsIPrompt doesn't have a right parent either, but it
will at least improve things. So it might be more of a workaround, then a real
fix.

ccarlen, is the exposure of the parent ok?

The rest of the patch is just to pass aPrompt around, into
nsICookiePromptService.

Comment 10

[[e-mail]]

Comment on attachment 122450 [details] [diff] [review]
first shot

This looks conceptually fine to me, but just as mvl said i have no idea if it's
right. ;)

we need ccarlen & darin to look at this (should this be on the radar for 1.4b
btw?)

Comment 11

[[e-mail]]

(r=dwitte on the cookie-related parts btw)

Comment 12

[Conrad Carlen (not reading bugmail)]

Comment on attachment 122450 [details] [diff] [review]
first shot

>Index: netwerk/base/public/nsIPrompt.idl
>===================================================================
> [scriptable, uuid(a63f70c0-148b-11d3-9333-00104ba0fd40)]
> interface nsIPrompt : nsISupports
> {
>@@ -121,4 +123,11 @@
>                    in  PRUint32 count,
>                    [array, size_is(count)] in wstring selectList,
>                    out long outSelection);
>+
>+    /**
>+     * The parent window for the dialogs. This can be used as a hack 
>+     * for where you have an nsIPrompt, but you need to create your 
>+     * own custom dialog as a child of the parent window.
>+     */
>+    readonly attribute nsIDOMWindow parent;
> };

We can't have such a hack built into an interface. Just in this patch, it makes
ugly things happen in the code. It's now passing around an nsIPrompt to
represent the parent window instead of an nsIDOMWindow? That's bad - they
shouldn't be related in this way.

This patch should work for somebody who needs to get around this *in their own
tree* but, I think we need to fix this for real in order to check it in.

Comment 13

[[e-mail]]

I agree completely that it's a hack, and neither me or mvl like it much... we
just couldn't see a better way.

darin: "if anyone is interested in helping, please let me know, and i'll explain
what i think needs to be done."

can we hear that explanation? ;)

Comment 14

[Conrad Carlen (not reading bugmail)]

I think the problem is that, at some point early in the process, we have an
nsIInterfaceRequestor but we use that just to get an nsIPrompt and then let go
of it. I think the interface requestor should be kept around and passed to
internal cookie methods which take an nsIPrompt. The interface requestor is more
general and can be used to get an nsIPrompt, nsIDOMWindow, and with some work,
an nsICookiePromptService.

http://lxr.mozilla.org/seamonkey/source/extensions/cookie/nsCookieHTTPNotify.cpp#233

Comment 15

[Darin Fisher]

so, my patch for bug 22994 makes it a lot easier to access the
nsIInterfaceRequestor since cookie method almost always now have access to a
channel (plugins being the only exception i think).  perhaps this will enable
you to do what conrad is suggesting.

Comment 16

[[e-mail]]

yup, I was thinking that. i think you've done half the job for us. ;)

conrad's comment coupled with what darin told mvl on irc a while back, gives us
enough info to fix this up... hopefully we'll have something soon.

Comment 17

[Michael Buckland]

Discussed in topembed bug triage.  Minusing.

Comment 18

[Mike Pinkerton (not reading bugmail)]

requesting re-triage.

Comment 19

[Samir Gehani]

adt: nsbeta1-

Comment 20

[Michael Dunn]

dwitte are you any  closer to a patch?

Comment 21

[saari (gone)]

topembed-

Comment 22

[Mike Pinkerton (not reading bugmail)]

*** Bug 190741 has been marked as a duplicate of this bug. ***

Comment 23

[[e-mail]]

*** Bug 213254 has been marked as a duplicate of this bug. ***

Comment 24

[[e-mail]]

marking dependency on bug 210561, since darin's patch should take care of most
cases.

Comment 25

[Michiel van Leeuwen (email: mvl+moz@)]

the work in bug 210561 seems to have fixed this for me (linux). 
From comment 5 this is more visible on macosX. Can anyone test if it is fixed there?

Comment 26

[baffclan]

I cannot reproduce with 2003122608-trunk/WinXP.

Comment 27

[Koike Kazuhiko]

-> WFM

Comment 28

[Michael Kurze [:michaelk]]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a) Gecko/20040425
Firefox/0.8.0+

MacOS X.3.3

WFM