166945 - Remove "pippki" from embedding packages

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P1)

Description

[Kai Engert (:KaiE:)]

pippki is the portion of PSM that provides Mozilla's XUL UI for PSM.

The embedding packages include file pippki.[dll|so]
This should not be necessary, and it's presence actually can cause problems in
some embedding environments.

I suggest to remove that component from the embedding package.
Please object if you know a reason why this shouldn't be done.

Comment 1

[Kai Engert (:KaiE:)]

cc'ing people who worked on bug 81884, since the pippki package was added with a
patch in that bug.

Comment 2

[Kai Engert (:KaiE:)]

Attachment: Patch v1

Comment 3

[Kai Engert (:KaiE:)]

Adam, can you please review?

Comment 4

[Adam Lock]

Comment on attachment 98028 [details] [diff] [review]
Patch v1

Does pippki only contain the front end and nothing else?

Assuming so, r=adamlock

Comment 5

[Alec Flett]

Comment on attachment 98028 [details] [diff] [review]
Patch v1

sr=alecf

Comment 6

[Kai Engert (:KaiE:)]

fixed on trunk

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This seems to have caused bug 169943.

Comment 8

[Chak Nanga]

Sorry, but i have to reopen this bug.

pikpki.dll/xpt seem to be needed in certain embedding scenarios as described below. 

We have a certain embedding customer who uses the following code fragment to
display the Cert details using their own native UI:

void ShowCertDetailInfo()
{
  nsIASN1Object *asn1Obj;
  nsCOMPtr<nsIASN1Tree> asn1Tree = do_GetService(NS_ASN1TREE_CONTRACTID);
  
  /* Without the pippki.dll/xpt present |asn1tree| var will be null and we fail 
     to proceed with displaying cert details
  */
...
...
  asn1Tree->LoadASN1Structure(asn1Obj);
  //Code Display asn1Obj here
}

Comment 9

[David P. Drinan]

Adding javi to cc-list.

Javi, 

Should we move NS_ASN1TREE_CONTRACTID implementation into pipnss?

Comment 10

[Kai Engert (:KaiE:)]

I disagree, unless there is a strong need for it.
David, do you have a need for that, too?

I will look at the code that Chak mentions and supply a way to not use the
implementation. I believe the current code does not really need it.

Comment 11

[David P. Drinan]

If the NS_ASN1TREE_CONTRACTID implementation is useful to embedders to write 
cert view code, then I think it should be part of the embedder kit. Of course, 
if there is an easier way for them to achieve the same result without have to 
move implementation and interfaces around, then that's fine too.

Comment 12

[Javier Delgadillo]

Kaie, 

I think we should go ahead and put this into the pipnss embeders module simply
because it's useful enough that many other embeders may want to use it. 
Otherwise, every embeder that wants to populate a tree with cert details like
the Mozilla cert viewer will have to write something very similar, if not
exactly, like our nsIASN1Object implementation.

Comment 13

[Kai Engert (:KaiE:)]

I think one of the arguments against doing that was, nsIASNTreeView is derived
from nsITreeView, which is XUL specific, and most embeddors are not using XUL.

Comment 14

[Javier Delgadillo]

Man this code keeps changing alot.  I thought we were talking about tne
nsIASN1Object.

I'm not so sure nsIASN1Tree should become a part of the default embeder module
unless nsITreeView is an interface embeders are free to use as well.

Comment 15

[Kai Engert (:KaiE:)]

I believe I have the embedding that Chak uses in front of me.
I believe it is a bad idea to use nsIASN1Tree for that embedding code.
What is done is the embedding code abuses Mozilla's tree UI implementation as
its own data structure for mapping "index within ASN1Sequence" to "pointer to
ASN1Object".

The embedding code has to maintain it's own mapping from embedding UI code to
actual data objects anyway, and the embedding code is already doing that
partially. I see a easy way to get rid of abusing this private Mozilla completely.

I'm closing this bug again and will work with Chak and provide more info.

Comment 16

[Kai Engert (:KaiE:)]

FYI: We forgot something, see new bug 188145.