217082 - Tree view of certificate fields in Certificate viewer is corrupt

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect)

Description

[danne.persson]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030718
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030718

Opening a mail that has been digitally signed (thawte personal freemail CA)
with a valid signature. open up the security info (double click on "pen")
view signature certificate --> details
the Certificate field tree can corrupt sometimes, making it possible to 
press the topmost "-" and make the complete tree disappear. also, just before
this happens the end of the tree disappears. Only way to get it
back, is to close the Certificate viewer, and reopen it using the "view
signature certificate --> details"



Reproducible: Sometimes

Steps to Reproduce:
1.Open a signed mail
2.view the "certificate viwer"
3. expand and compact the tree by pressing the + and - on the tree

how to reproduce it?
tried with 2 different certificates (a swedish bank certificate)
Can't understand exactly when it happens sadly.
It might be connected to the first few opening and closings of
the "leafs" if it doesn't happen then, it won't happen (~10 tries)
order of opening? how the tree expands out of view at the bottom limit?

On a side? issue. After a couple of opening and closings the tree also starts
showing gray lines to entries after the last ones that aren't available.

Actual Results:  
corrupt tree

Expected Results:  
The tree should not corrupt

Can always send screen caps of what it looks like.

Comment 1

[Daniel de Wildt]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20030916

I don't know if this is the same, but this also happens when showing SSL
certificates in the browser. An older bug (140302) with the certificate viewer
was assigned to the "PSM/Client library" component. Maybe you should correct
product/component fields of this bug.

Comment 2

[Daniel de Wildt]

Attachment: Screenshot of corrupted certificate viewer

The 'certificate fields' are corrupt (no scrollbar).
Clicking on the both remaining [-] (arrow down) will remove the corresponding
subtree and line. The line www.verisign.com will disappear after clicking in
the 'field value'.

Comment 3

[Daniel de Wildt]

This bug is still reproducible with
Mozilla/5.0 (Windows; U; Windows NT 5.0; de-AT; rv:1.6b) Gecko/20031125
=> new

The easiest way to reproduce this bug is by viewing the certificate full
expanded and the last [-] is on the last line in the view. The screenshot is the
result after collapsing the subtrees successive.

Comment 4

[Koobs]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031207
Firebird/0.7+

Confirming bug in above ^ build.

Steps to Reproduce:

1) Goto www.cacert.org
2) Tools > Page Info > Security > View > Details
3) Click "www.cacert.org" in 'Certificate Hierarchy'
4) Close the "[-] Certificate" node in 'Certificate Fields'
5) Close the "[-] www.cacert.org" node in 'Certificate Fields'

Result: Tree will disappear from 'Certificate Fields'

Comment 5

[Daniel de Wildt]

I have tested this with a 1.4 release (20030624) and the certificate viewer
works fine, so this is a regression. It would be fine if someone with a nightly
between the 1.4 and 1.5a releases could refine the date of this regression.

Comment 6

[Daniel de Wildt]

I think I found a bug in nsNSSASN1Tree::ToggleOpenState
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/manager/pki/src/nsASN1Tree.cpp&rev=1.15&cvsroot=/cvsroot&mark=366,370#351

CountVisibleNodes(n) counts the number of activated lines in AND after node n.
node n is the selected node with the clicked sign [+] or [-]. When open or
collapse a node, then I expect that only the numbers of rows BELOW this node
changes and not the nodes after this node.
Maybe that the following changes for the marked lines will work (I will test
after the weekend):
   rowCountChange = -CountVisibleNodes(n->seq);
   rowCountChange = CountVisibleNodes(n->seq);

Comment 7

[Daniel de Wildt]

Replacing the marked lines in
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/manager/pki/src/nsASN1Tree.cpp&rev=1.15&cvsroot=/cvsroot&mark=365-371#351
by
  if (IsExpanded) {
    rowCountChange = 0-CountVisibleNodes(n->child);
    n->seq->SetIsExpanded(PR_FALSE);
  } else {
    n->seq->SetIsExpanded(PR_TRUE);
    rowCountChange = CountVisibleNodes(n->child);
  }
will fix the testcases as described in comment 4 and comment 2.
I have no CVS access from here and patchmaker will not work on sourcecode files
without CVS access. Maybe someone could create a patch 
=> Setting keyword helpwanted

Comment 8

[John G. Myers]

Attachment: Proposed fix

Comment 9

[Stephane Saux]

r=ssaux

Comment 10

[John G. Myers]

Comment on attachment 141549 [details] [diff] [review]
Proposed fix

(Recording ssaux review)

Comment 11

[(not reading, please use [e-mail] instead)]

Comment on attachment 141549 [details] [diff] [review]
Proposed fix

sr=sspitzer, sorry for the delay john.

Comment 12

[John G. Myers]

Fix checked in.  Thanks for the help.