Name modification isn't updated immediately

VERIFIED FIXED in mozilla0.9.2

Status

SeaMonkey
MailNews: Address Book & Contacts
P3
normal
VERIFIED FIXED
17 years ago
14 years ago

People

(Reporter: yulian chang, Assigned: Srilatha Moturi)

Tracking

(Blocks: 1 bug)

Trunk
mozilla0.9.2
x86
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: Have a fix,r=,sr=,a=)

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
Netscape 6.5/2001050914 build

1. Modify Name in New Directory| General
2. OK  to New Directory dialog
3. The Name listed in LDAP Directory Servers dialog is not updated
4. Close out LDAP Directory Servers dialog
5. Click on Edit Directories button
6. The updated Name then shows up

Updated

17 years ago
QA Contact: esther → yulian
(Assignee)

Updated

17 years ago
Priority: -- → P2
Target Milestone: --- → mozilla0.9.2

Updated

17 years ago
Blocks: 17880
Priority: P2 → P3
(Assignee)

Comment 1

17 years ago
Created attachment 36734 [details] [diff] [review]
patch v1
(Assignee)

Comment 2

17 years ago
ccing Bhuvan for review
ccing Seth for sr
Status: NEW → ASSIGNED
Whiteboard: Have a fix, requesting review
(Assignee)

Comment 3

17 years ago
now ccing

Comment 4

17 years ago
r=mohanb; 

Comment 5

17 years ago
+    dump("gNewServer is " + gNewServer + "\n"); 

Srilatha, please don't add new dumps to the tree.
(Assignee)

Comment 6

17 years ago
Created attachment 37419 [details] [diff] [review]
New patch without the dump

Comment 7

17 years ago
ok, r=mohanb;
(Assignee)

Updated

17 years ago
Whiteboard: Have a fix, requesting review → Have a fix, r=, need an sr=
+  if(gRefresh) 
+  { 
+    var directoriesTree = document.getElementById("directoriesTree"); 
+    var selectedNode = directoriesTree.selectedItems[0]; 
+    var row  =  selectedNode.firstChild; 
+    var cell =  row.firstChild; 
+    cell.setAttribute('label', gNewServer); 
+    cell.setAttribute('string', gNewServerString);
+    window.opener.gRefresh = true; 
+  } 


it seems like that should be:

+    window.opener.gRefresh = false;
+  }

or am I confused?
Whiteboard: Have a fix, r=, need an sr= → Have a fix, requesting review
(Assignee)

Comment 9

17 years ago
+  if(gRefresh) 
+  { 
+    var directoriesTree = document.getElementById("directoriesTree"); 
+    var selectedNode = directoriesTree.selectedItems[0]; 
+    var row  =  selectedNode.firstChild; 
+    var cell =  row.firstChild; 
+    cell.setAttribute('label', gNewServer); 
+    cell.setAttribute('string', gNewServerString);
//You are right. here I need to have gRefresh = false;
+    window.opener.gRefresh = true; 
but the window.opener is for either Addressing Panel in global preferences
or ServerSettings panel in Mail/News Account Settings. 
I need to have window.opener.gRefresh = true so that the dropdown list box in 
the above mentioned panels will be refreshed.
Will post a patch with the change right away.
(Assignee)

Comment 10

17 years ago
Created attachment 37430 [details] [diff] [review]
updated patch
I *think* I understand what you're doing here, but it's pretty confusing.

you should comment this code so it's clear why you set gRefresh to false, but
window.opener.gRefresh to true.

having them be the same name (gRefresh) adds to the confusion.  I recommend
renaming these so it's clear what they are going to refresh.

another, cleaner approach would be to have a method that does the "refresh",
instead of setting globals like this.
(Assignee)

Comment 12

17 years ago
Created attachment 37447 [details] [diff] [review]
updated patch
(Assignee)

Comment 13

17 years ago
Sorry, for the confusion. I have added comments and also updated one gRefresh to 
gUpdate.
sr=sspitzer, assuming mohanb is ok with it.

Comment 15

17 years ago
r=mohanb;
(Assignee)

Updated

17 years ago
Whiteboard: Have a fix, requesting review → Have a fix,r=,sr=, need a=

Updated

17 years ago
Blocks: 83989
a=dbaron for trunk checkin (on behalf of drivers)
Whiteboard: Have a fix,r=,sr=, need a= → Have a fix,r=,sr=,a=
(Assignee)

Comment 17

17 years ago
Fix checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 18

17 years ago
Verified with 2001061304 winds trunk build
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.