Closed
Bug 149841
Opened 22 years ago
Closed 21 years ago
It should NOT be possible to open cert manager twice at the same time
Categories
(Core Graveyard :: Security: UI, defect, P3)
Tracking
(Not tracked)
VERIFIED
FIXED
Future
People
(Reporter: KaiE, Assigned: KaiE)
References
Details
Attachments
(1 file, 2 obsolete files)
3.15 KB,
patch
|
iamawalrus
:
review+
alecf
:
superreview+
|
Details | Diff | Splinter Review |
Go to Edit/Prefs/Security/Certificates. Click "manage certificates". The certificate manager opens. Move away the new window, but do not close it. Find the open prefs window. Click again on "manage certificates". Actual behaviour: Another certificate manager window will open. If you modify one window by deleting or importing certs, the other open cert cert window will not disappear. (In addition, it will most like crash, when you delete a cert in one window, and try to work with that cert in the other open window). Expected behaviour: When you click the "manager certificates" button again, there should not open another window. Instead, the already open window should come to the front.
Assignee | ||
Comment 1•22 years ago
|
||
Forgot the word NOT in the summary, adding.
Summary: It should be possible to open cert manager twice at the same time → It should NOT be possible to open cert manager twice at the same time
Assignee | ||
Comment 2•22 years ago
|
||
-> me
Assignee | ||
Comment 3•22 years ago
|
||
Assignee | ||
Comment 4•22 years ago
|
||
Javi, can you please review?
Comment 5•22 years ago
|
||
Comment on attachment 93001 [details] [diff] [review] Patch v1 does calling focus really make it so that you can't open a new one? Shouldn't we look at making the window modal instead?
Assignee | ||
Comment 6•22 years ago
|
||
Calling focus is just a secondary thing to make it work. I do it to bring an already open window to front. The real trick is the changed window.open() call. By giving the window a name, subsequent calls to window.open() will re-use the previous window, and not open a second one.
Comment 7•22 years ago
|
||
Comment on attachment 93001 [details] [diff] [review] Patch v1 In that case, r=javi
Assignee | ||
Comment 8•22 years ago
|
||
Comment on attachment 93001 [details] [diff] [review] Patch v1 marking patch as has-review
Attachment #93001 -
Flags: review+
Assignee | ||
Comment 9•22 years ago
|
||
Alec, can you please review?
Comment 10•22 years ago
|
||
Comment on attachment 93001 [details] [diff] [review] Patch v1 hmm.. can't you just make this modal? I'm not sure how focusing the window will have the desired effect.
Assignee | ||
Comment 11•22 years ago
|
||
Alec, I believe it was a design decision to make the certificate non modal. I was not involved in that decision, it happened before I worked on PSM. We would need to find the people involved in the original decision, but I believe making it modal is not wanted. My patch is just a small effort to make the existing behaviour more robust. Alec, if you know of a function "bringToFront()", please let me know. I could not find any, but it would probably more sense. At least on Linux, this function brought the window to front when hidden behind other windows. In any way, the major motivation is to avoid that certificate manager gets opened multiple times.
Comment 12•22 years ago
|
||
all I mean is that the focus stuff in the patch doesn't do anything - newly opened windows already have focus, and this doesn't prevent me from opening a second window. Ah, now I see why it appears that you avoided opening a new window! It has nothing to do with the w.focus(). The reason is that the patch adds the window target in openCertManager() - so that when you call window.open, the window will re-open in the same chrome window..a good test would be to change something in the window without saving anything, and then try to open the window again - what you should find is that any changes you made were reset. I think you're going to have to cache the result of window.open or something, and clear it when the window closes. You could also try the window cycling code that the taskbar uses (the mozilla taskbar, not the windows taskbar) to find the most recently used window of a given type.
Assignee | ||
Comment 13•22 years ago
|
||
*** Bug 162034 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 14•22 years ago
|
||
I agree. Probably the most similar code I can re-use will be the logic for the preferences window, since it is modeless, too.
Assignee | ||
Updated•22 years ago
|
Priority: -- → P3
Target Milestone: --- → Future
Comment 15•21 years ago
|
||
this patch finds the most resent opened window and focuses on it.
Attachment #115351 -
Flags: review?(kaie)
Assignee | ||
Comment 16•21 years ago
|
||
Comment on attachment 115351 [details] [diff] [review] patch Great patch, thanks! I already tried it, it works for me. r=kaie
Assignee | ||
Comment 17•21 years ago
|
||
Comment on attachment 93001 [details] [diff] [review] Patch v1 marking my earlier patch as obsolete
Attachment #93001 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #115351 -
Flags: review?(kaie) → review+
Comment 18•21 years ago
|
||
only modifies the comment a little since I directly copied the code from the preference window code.
Attachment #115351 -
Attachment is obsolete: true
Comment 19•21 years ago
|
||
Comment on attachment 115353 [details] [diff] [review] patch take over r=kaie and seeking sr=?
Attachment #115353 -
Flags: superreview?(alecf)
Attachment #115353 -
Flags: review+
Comment 20•21 years ago
|
||
Comment on attachment 115353 [details] [diff] [review] patch sr=alecf
Attachment #115353 -
Flags: superreview?(alecf) → superreview+
Comment 21•21 years ago
|
||
checked in. Thanks!
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•