243807 - profile manager hangs other dialogs' action

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Nian Liu(n/a in a long time)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4.1) Gecko/20040302
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4.1) Gecko/20040302

open profile manager, open password manager or other manager. any operation on
password/other manager can't be triggered. it's also in moz1.7/trunk, and on solaris

Reproducible: Always
Steps to Reproduce:
1.from "tools" menu to open profile manager
2.from "tools" menu to open password manager


Actual Results:  
operation on password manager can't be triggered

Expected Results:  
operation on password manager should be triggered

Comment 1

[Nian Liu(n/a in a long time)]

Attachment: patch for 243807

openWindow should be without "modal"

Comment 2

[[e-mail]]

Comment on attachment 149260 [details] [diff] [review]
patch for 243807

This code was probably translated from the C++ profile startup code, which does
need it to be modal...

Comment 3

[[e-mail]]

Comment on attachment 149260 [details] [diff] [review]
patch for 243807

Mind you, it would be nice if you registered a window type for the profile
selection window so that you could raise an existing window.

Comment 4

[[e-mail]]

Comment on attachment 149260 [details] [diff] [review]
patch for 243807

Oh, and you could probably use window.open instead of ww.openWindow too...

Comment 5

[Nian Liu(n/a in a long time)]

Attachment: patch for 243807

add register mechanism to keep from creating more than one instance

Comment 6

[Nian Liu(n/a in a long time)]

Comment on attachment 149391 [details] [diff] [review]
patch for 243807

neil, 
new attach with register mechanism. pls. help me review it. and if there are
any other way to register, pls. point them out. btw, if that's ok, could you
please help me recommend a guy for sr?

Comment 7

[[e-mail]]

Comment on attachment 149391 [details] [diff] [review]
patch for 243807

>+  var enumerator = wm.getEnumerator("mozilla:profileSelection");
>+  while (enumerator.hasMoreElements()) {
>+    var viewer = enumerator.getNext();
>+    if (viewer.arguments[0] == viewerType) {
>+      viewer.focus();
>+      return;
>+    }
>+  }
What's viewerType? It doesn't appear to be defined. I can't actually tell what
that test is for, otherwise I would suggest using wm.getMostRecentWindow
instead.

>+  window.openDialog( "chrome://communicator/content/profile/profileSelection.xul",
>                 null,
I missed this last time, but null is an invalid value here (it gets converted
to "null"). Ideally openDialog should ignore the value when using "chrome" as a
flag, but in the mean time please use "" or "_blank".
>-                "centerscreen,chrome,modal,titlebar",
>+                "centerscreen,chrome,titlebar",
>                 params);

Comment 8

[Nian Liu(n/a in a long time)]

Attachment: patch for 243807

sorry, need strong coffee

Comment 9

[Nian Liu(n/a in a long time)]

Comment on attachment 149461 [details] [diff] [review]
patch for 243807

thanks again

Comment 10

[[e-mail]]

Comment on attachment 149461 [details] [diff] [review]
patch for 243807

>+  if( promgrWin ){
>+    promgrWin.focus();
>+  }else{
Nit: Ugly inline spacing, it should look like this:
if (promgrWin) {
  promgrWin.focus();
} else {
No need for a new patch, just get this changed before check in.

Comment 11

[Nian Liu(n/a in a long time)]

Attachment: patch for 243807 after review

code style modification

Comment 12

[Johnny Stenback (:jst)]

Comment on attachment 149658 [details] [diff] [review]
patch for 243807 after review

+  if(promgrWin) {

I believe Neil wanted a space after the 'if' as well.

sr=jst, and you've tested that this doesn't break the profile manager on
startup (mozilla -P), right?

Comment 13

[Nian Liu(n/a in a long time)]

yes, it's a different scenario and won't exec this code.

Comment 14

[Nian Liu(n/a in a long time)]

Attachment: patch for 243807(code style modification)

code style modification, more...

Comment 15

[Kyle Yuan]

Fixed!

Checking in xpfe/communicator/resources/content/tasksOverlay.js;
/cvsroot/mozilla/xpfe/communicator/resources/content/tasksOverlay.js,v  <-- 
tasksOverlay.js
new revision: 1.56; previous revision: 1.55
done
Checking in profile/resources/content/profileSelection.xul;
/cvsroot/mozilla/profile/resources/content/profileSelection.xul,v  <-- 
profileSelection.xul
new revision: 1.54; previous revision: 1.53
done