Closed
Bug 173844
Opened 22 years ago
Closed 20 years ago
Cookie confirmation dialog should be a child of originating window
Categories
(Core :: Networking: Cookies, defect, P3)
Core
Networking: Cookies
Tracking
()
RESOLVED
WORKSFORME
mozilla1.4beta
People
(Reporter: kazhik, Assigned: darin.moz)
References
Details
(Keywords: regression, topembed)
Attachments
(1 file)
8.75 KB,
patch
|
ccarlen
:
review-
|
Details | Diff | Splinter Review |
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.
Updated•22 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P4
Target Milestone: --- → mozilla1.3beta
Assignee | ||
Comment 1•22 years ago
|
||
*** Bug 170172 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 2•22 years ago
|
||
*** Bug 188566 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•22 years ago
|
Target Milestone: mozilla1.3beta → Future
Comment 4•21 years ago
|
||
*** Bug 199985 has been marked as a duplicate of this bug. ***
Comment 5•21 years ago
|
||
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.
Severity: normal → major
Keywords: regression,
topembed
Comment 6•21 years ago
|
||
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.
Updated•21 years ago
|
OS: Linux → All
Hardware: PC → All
Assignee | ||
Comment 7•21 years ago
|
||
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.
Assignee | ||
Updated•21 years ago
|
Status: NEW → ASSIGNED
Priority: P4 → P2
Target Milestone: Future → mozilla1.4beta
Assignee | ||
Updated•21 years ago
|
Priority: P2 → P3
Comment 9•21 years ago
|
||
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•21 years ago
|
||
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?)
Attachment #122450 -
Flags: superreview?(darin)
Attachment #122450 -
Flags: review?(ccarlen)
Comment 11•21 years ago
|
||
(r=dwitte on the cookie-related parts btw)
Comment 12•21 years ago
|
||
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.
Attachment #122450 -
Flags: review?(ccarlen) → review-
Comment 13•21 years ago
|
||
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? ;)
Updated•21 years ago
|
Attachment #122450 -
Flags: superreview?(darin)
Comment 14•21 years ago
|
||
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
Assignee | ||
Comment 15•21 years ago
|
||
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•21 years ago
|
||
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•21 years ago
|
||
Discussed in topembed bug triage. Minusing.
Comment 18•21 years ago
|
||
requesting re-triage.
Comment 21•21 years ago
|
||
topembed-
Comment 22•21 years ago
|
||
*** Bug 190741 has been marked as a duplicate of this bug. ***
Comment 23•21 years ago
|
||
*** Bug 213254 has been marked as a duplicate of this bug. ***
Comment 24•21 years ago
|
||
marking dependency on bug 210561, since darin's patch should take care of most cases.
Depends on: 210561
Comment 25•21 years ago
|
||
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•21 years ago
|
||
I cannot reproduce with 2003122608-trunk/WinXP.
Reporter | ||
Comment 27•20 years ago
|
||
-> WFM
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → WORKSFORME
Comment 28•20 years ago
|
||
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
You need to log in
before you can comment on or make changes to this bug.
Description
•