Cookie manager: multiple windows can be opened

VERIFIED FIXED in mozilla1.6alpha

Status

()

--
minor
VERIFIED FIXED
16 years ago
14 years ago

People

(Reporter: benjamin, Assigned: darin.moz)

Tracking

Trunk
mozilla1.6alpha
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: checklinux checkwin)

Attachments

(2 attachments)

(Reporter)

Description

16 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC; en-US; rv:1.3a) Gecko/20021127
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC; en-US; rv:1.3a) Gecko/20021127

If the cookie manager window is already opened and is in background, if I select
Tools > Cookie Manager > Manage Stored Cookie a new cookie manager window opens.

Reproducible: Always

Steps to Reproduce:
1. Open the cookie manager (Tools > Cookie Manager > Manage Stored Cookie)
2. Bring a Navigator window in foreground
3. Select again Tools > Cookie Manager > Manage Stored Cookie

Actual Results:  
Now you should have two Cookie Manager windows.

Expected Results:  
The first Cookie Manager windows should gets focus instead of having a new
Cookie Manager window.

MacOS 8.6

Comment 1

16 years ago
Reproducible on Mac OS 9.1, Mozilla 1.2.1

Comment 2

16 years ago
confirming but I don't think this will cause any problems.  You can still manage
the cookie list properly.  Win NT trunk.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Mac System 9.x → All
Hardware: Macintosh → All
(Assignee)

Comment 3

16 years ago
-> suresh
Assignee: morse → suresh
[Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.3) Gecko/20030312]

Bug still there.

Without testing, I would agree with comment 2:
*You may want to change the Severity from 'Normal' to 'Minor':
 Workaround is to close the new window and manually focus the previous one.

Same bug with:
*Tools: Form M., Image M., Password M.
 *You may want to update Summary to include all keywords;
  And possibly to change the Component.

No bug (== Allways uses one "window" only):
*Edit: Preferences...
*Tools: Download M., Web Dev. > JavaScript Console

This reminds me of ("unrelated") bug 172537 comment 3.

*NB: 'Tools > Popup M.' does not create a window icon in the task bar, nor allow
to focus back to the Browser window which was used to open it :-(
 *Should I file a new (I do not find an existing one) bug for this, or could it
be considered a part of the current bug ?

Comment 5

16 years ago
Mozilla 1.f - all plats.
mvl on IRC pointed out that Cookie Manager has common code w/ Image Blocking,
but the other managers might be independent.

If you see that in other managers, you should file a bug against them in their
component.

If there is a common, underlying bug (some XUL template), then I'll mark this
bug as dependent.
QA Contact: tever → benc
Summary: Multiple cookie manager windows can be opened → Cookie manager: multiple windows can be opened

Comment 6

16 years ago
To do this for a non-modal window you first need to give it a windowtype
attribute. Having done this you can then use the window watcher to search for an
existing instance of the window:

function getMostRecentWindow( inType )
{
  const kWindowMediatorContractID = "@mozilla.org/appshell/window-mediator;1";
  const kWindowMediatorIID = Components.interfaces.nsIWindowMediator;
  const kWindowMediator = Components.classes[kWindowMediatorContractID]
.getService(kWindowMediatorIID);
  return kWindowMediator.getMostRecentWindow( inType );
}

If you think that the user might try to open the window twice in quick
succession (the windowtype won't be immediately valid after window.open) then
you need to save the window you have started to open as a flag and then clear it
when the window loads:

function toNewWindow(uri)
{
  // Recently opened one.
  if (uri in window)
    return;

  function newWindowLoaded(event) {
    delete window[uri];
  }
  window[uri] = window.openDialog(uri, "_blank", "chrome,all,dialog=no");
  window[uri].addEventListener("load", newWindowLoaded, false);
}

Alternatively if you need to be able to open the window with different
parameters you have a choice: either having found a window call a function to
update it, or close that window and open a new one, i.e.

function viewCookies() {
  var viewer = getMostRecentWindow( "manage:permissions" );
  if (viewer)
    viewer.close();

  window.openDialog(/* stuff */);
}

Hope this helps.
Reply to comment 5: (on my comment 4)

*Popup Manager issue:
 *See now bug 198250 which someone else filed in the meantime.

*I filed the 3 following bugs: bug 199495 (F.M.), bug 199497 (I.M.), bug 199498
(Pa.M.).
 *I suspect a common underlying issue: but I did not find such a bug report, and
I don't have any clue on it to file one yet.

Comment 8

16 years ago
Nominating...
Keywords: nsbeta1
[Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.4a) Gecko/20030401]

Bug still there. (with v1.3 profile, at least)

Comment 10

16 years ago
mvl, do you want to take this?

Comment 11

16 years ago
adt: nsbeta1-
Keywords: nsbeta1 → nsbeta1-
[Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.4b) Gecko/20030507]

Bug still there. (with v1.3 profile, at least)

Comment 13

16 years ago
There's a patch for the Download Manager in bug 131762. May be useful if both
managers are using similar code.

Comment 14

15 years ago
The patch in Bug #131762
(http://bugzilla.mozilla.org/attachment.cgi?id=79221&action=view) looks simple
and clean. I vote for moving the code that was added to nsDownloadManager.cpp to
the equivalent Cookie Manager program (with the appropriate changes of course).

If I can get some of my own outstanding patches cleared up, I might even be able
to help with this.

Comment 15

15 years ago
This patch:

Index: extensions/cookie/resources/content/cookieOverlay.js
===================================================================
RCS file: /cvsroot/mozilla/extensions/cookie/resources/content/cookieOverlay.js,v
retrieving revision 1.14
diff -u -r1.14 cookieOverlay.js
--- extensions/cookie/resources/content/cookieOverlay.js	26 Mar 2003 02:02:42
-0000	1.14
+++ extensions/cookie/resources/content/cookieOverlay.js	25 Aug 2003 14:25:53 -0000
@@ -27,7 +27,7 @@
 }
 
 function viewCookies() {
- 
window.openDialog("chrome://communicator/content/wallet/CookieViewer.xul","_blank",
+ 
window.openDialog("chrome://communicator/content/wallet/CookieViewer.xul","_cookieManager",
                     "chrome,resizable=yes", "cookieManager");
 }  

is much simpler and will suffice. It has its drawbacks but it's a technique used
throughout the codebase.

If you do go with the WindowMediator version,

+  nsCOMPtr<nsIWindowMediator> wm =
do_GetService("@mozilla.org/rdf/datasource;1?name=window-mediator", &rv);
+  if (NS_FAILED(rv)) return rv;

oy! just do

nsCOMPtr<nsIWindowMediator> wm(do_GetService(NS_WINDOWMEDIATOR_CONTRACTID));
if (wm) {
  ...
}
(Assignee)

Comment 16

15 years ago
-> me

danm: what are the drawbacks of the _cookieManager approach?
Assignee: suresh → darin
Severity: normal → minor
Target Milestone: --- → mozilla1.6alpha

Comment 17

15 years ago
I thought chrome and user script shared the same namespace, so if a window
opened by a webpage happened to have the same name, they'd collide. But it looks
like that's not happening. I wonder if someone fixed that. Nifty.

However my solution ends up reloading the cookie manager window when you
reselect its menu item, and this seems to clear out the dialog's contents.
Alright I take it back, the windowname method won't suffice. Guess you have to
go with the WindowMediator version in this case. That's a shame. The windowname
method is much simpler and supposed to work. Looks like it could stand a little
more tweaking, though.
(Assignee)

Comment 18

15 years ago
danm: thanks for the info... care to attach a patch? ;-)

Comment 19

15 years ago
Created attachment 132683 [details] [diff] [review]
use windowmediator to focus extant cookie viewer

Yeah, if you don't mind waiting for it.

The reason reloading an extant cookie manager window clears out its contents is
bug 25040. Even were that fixed though, it seems suboptimal to reload the
window. So Neil's plan (comment 6) is probably best. However I don't think you
need to worry about the load listener or parameter change cases.

Since the cookie and image viewers are both the same XUL, a simple plan to
focus the most recently used window of that type will ... well, you know. Seems
best to me to keep track with both windowtype and name, as in the attached
patch. And while I was there I also reformatted to 80-space lines (*love* those
unGUI editors) and got rid of the two var declarations up top. They should be
consts but that doesn't work for some reason that I'll let you worry about if
you want. They were unused, anyway.

Comment 20

15 years ago
There is a problem with the above patch. Should the user already have the Cookie
Manager open and then click the Privacy icon that becomes visible in the
browser's statusbar after a cookie is rejected, this patch will prevent the
Cookie Manager from hiding the Privacy icon and forcing its own Status column to
be visible (and sorting on it). Heh heh. Back to you, Bob.

Comment 21

15 years ago
I've since seen how Help does it. If you have more than one Help source file
(perhaps you install an extension with its own help, such as xulmine) then each
help file opens in its own window, but if you request further help then the
correct window is reused. I think it should be possible to adapt the Help code
to resolve this issue too.

Comment 23

15 years ago
Comment on attachment 132722 [details] [diff] [review]
Possible patch

great stuff! r=me
Attachment #132722 - Flags: review+

Updated

15 years ago
Attachment #132722 - Flags: superreview?(darin)

Comment 24

15 years ago
That's identical in function to my patch. It's the same, except you
differentiate between the different kinds of viewers using window.arguments
instead of window.name. Comment 20 still applies.

Comment 25

15 years ago
hmm, won't it open a new window with the correct "fromIcon" argument?

it might not be neat, but i'm ok with that for now... i don't really like that
icon anyway, hopefully we'll kill it soon :)

Comment 26

15 years ago
Oh. You're so right. That does make it slightly different from my patch. However
that doesn't get rid of the problem; it makes the problem different. With this
patch, then, you still can have those two copies of the Cookie Manager dialog
open simultaneously.
(Assignee)

Comment 27

15 years ago
Comment on attachment 132722 [details] [diff] [review]
Possible patch

sr=darin
Attachment #132722 - Flags: superreview?(darin) → superreview+

Comment 28

15 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 29

14 years ago
V/fixed:
mozilla 1.84a, mac os x.

Still brings up multiple versions as described by DanM in #26. New bug?
Status: RESOLVED → VERIFIED
Whiteboard: checklinux checkwin
You need to log in before you can comment on or make changes to this bug.