Closed
Bug 82886
Opened 23 years ago
Closed 23 years ago
[CertManager] Title selection popup has extra checkbox
Categories
(Core Graveyard :: Security: UI, defect, P2)
Tracking
(Not tracked)
VERIFIED
FIXED
psm2.1
People
(Reporter: lord, Assigned: inactive-mailbox)
References
Details
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
In the Certificate Manager, you can select which titles are displayed. At the
end of the list there is a blank space with a check box next to it. This box
does not seem to control anything.
This extra checkbox should be removed.
Keywords: nsenterprise
Comment 2•23 years ago
|
||
t->2.1
kai
Assignee: ddrinan → kai.engert
Priority: -- → P2
Target Milestone: Future → 2.1
Updated•23 years ago
|
Target Milestone: 2.1 → Future
Comment 3•23 years ago
|
||
removing nsenterprise keyword from PSM bugs with target milestone of future.
Keywords: nsenterprise
Comment 4•23 years ago
|
||
Comment 5•23 years ago
|
||
<outliner> seems to create the the column picker out of the specified
<outlinercol>'s so the column picker doesn't need to be declared in each .xul
file
Assignee | ||
Comment 6•23 years ago
|
||
There is code in the C++ files, that fills the cell with an internal certificate
key. My assumption is, this was used solely for debugging purposes.
However, the original programmer assumed that using the collapsed="true"
attribute hides the column completly. As we know now, this is not true for the
outliner's column picker. It still shows up there.
The solution is to remove the column from XUL completely. However, as this might
be a very helpful debugging feature for future programmers working on this code,
I'm suggesting to deactivate the code by commenting it out (and not removing
it). For completeness, I'm commenting out the C++ code that provides the cell
contents as well.
Ian, can you please review the next patch?
Assignee | ||
Comment 7•23 years ago
|
||
Comment 8•23 years ago
|
||
Actually, the db key field is there from an early implementation of the cert
outliner view. IIRC, I first thought storing an array of all the certs
displayed would cost too much memory. Thus I only stored the db key (via a
hidden field in the outliner view), and used that to look up the cert as needed.
For reasons I don't recall at the moment, I changed my mind, and decided to
keep an array of certs, thus removing the need for the db key field (though I
apparently forgot to actually remove the code ;).
For this bug, commenting out the db key code is fine. However, you might
consider the larger design issues. When PSM/mozilla supports S/MIME, a user
could potentially have 100's of certs. Thus keeping them all in memory would be
bad. Rather, only the parts needed for the outliner display (nickname or email,
verified, validity, etc.) should be kept in memory, and when the user tries to
view the rest of the cert, a lookup is done using the db key.
Actually, even in that case, the db key should be stored along with the
aforementioned fields. It should not need to be part of the view at all. To be
more clear, I imagine something like this:
struct outlinerCertDisplay {
PRUnichar *displayName; /* nickname or email */
PRUnichar *validity;
...
char *dbkey;
}
Those fields are accessed by nsCertOutliner::GetCellText(), with the exception
of dbkey. It is only used for nsCertOutliner::GetCert().
Thus I think you should completely remove all references to the db key column in
the XUL files. The underlying implementation should not have to depend on that
hidden field.
Assignee | ||
Comment 9•23 years ago
|
||
Thanks for explaining.
I looked through the sources and couldn't find any other occurrences of the
column we want to remove.
I agree with your ID of the minimalistic in memory structure, which would reduce
memory usage and increase performance for larger lists.
I also see the advantage of having the db ID of a cert available. In the future,
we could have this temporary items stored in an array where array index equals
(expanded) outliner entry index, so we have access to the key for the selected
row without storing it in a cell. And we could put this logic into
nsIOutlinerView::getRowProperties.
Would you like to add r= for now?
Comment 10•23 years ago
|
||
yes, r=mcgreer
Comment 11•23 years ago
|
||
sr=blizzard
Comment 13•23 years ago
|
||
Checked in patch for Kai.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 14•23 years ago
|
||
*** Bug 94207 has been marked as a duplicate of this bug. ***
Comment 15•23 years ago
|
||
I see not yon checkbox namethed...
Marking VERIFIED FIXED on:
-MacOS91 2001-08-15-08-trunk (commercial)
-MacOS_X 2001-08-15-05-trunk (commercial)
-LinRH62 2001-08-15-08-trunk (commercial)
-Win98SE 2001-08-15-06-trunk (commercial)
Status: RESOLVED → VERIFIED
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
•