The default bug view has changed. See this FAQ.

It should NOT be possible to open cert manager twice at the same time

VERIFIED FIXED in Future

Status

Core Graveyard
Security: UI
P3
normal
VERIFIED FIXED
15 years ago
6 months ago

People

(Reporter: kaie, Assigned: kaie)

Tracking

Other Branch
Future

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

3.15 KB, patch
Robin Lu
: review+
Alec Flett
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

15 years ago
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

15 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)

Updated

15 years ago
Keywords: nsbeta1
(Assignee)

Comment 2

15 years ago
-> me
Assignee: ssaux → kaie
Keywords: nsbeta1 → nsbeta1+
(Assignee)

Comment 3

15 years ago
Created attachment 93001 [details] [diff] [review]
Patch v1
(Assignee)

Comment 4

15 years ago
Javi, can you please review?

Comment 5

15 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

15 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

15 years ago
Comment on attachment 93001 [details] [diff] [review]
Patch v1

In that case, r=javi
(Assignee)

Comment 8

15 years ago
Comment on attachment 93001 [details] [diff] [review]
Patch v1

marking patch as has-review
Attachment #93001 - Flags: review+
(Assignee)

Comment 9

15 years ago
Alec, can you please review?

Comment 10

15 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

15 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

15 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

15 years ago
*** Bug 162034 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 14

15 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

15 years ago
Priority: -- → P3
Target Milestone: --- → Future

Comment 15

14 years ago
Created attachment 115351 [details] [diff] [review]
patch

this patch finds the most resent opened window and focuses on it.

Updated

14 years ago
Attachment #115351 - Flags: review?(kaie)
(Assignee)

Comment 16

14 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

14 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

14 years ago
Attachment #115351 - Flags: review?(kaie) → review+

Comment 18

14 years ago
Created attachment 115353 [details] [diff] [review]
patch

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

14 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

14 years ago
Comment on attachment 115353 [details] [diff] [review]
patch

sr=alecf
Attachment #115353 - Flags: superreview?(alecf) → superreview+

Comment 21

14 years ago
checked in. Thanks!
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 22

14 years ago
Verified fixed.
Status: RESOLVED → VERIFIED

Updated

12 years ago
Component: Security: UI → Security: UI
Product: PSM → Core
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.