search

kaikramer / keystore-explorer

KeyStore Explorer is a free GUI replacement for the Java command-line utilities keytool and jarsigner.
https://keystore-explorer.org/
GNU General Public License v3.0
1.67k stars 271 forks source link

Make view certificate dialog non-modal #403

Closed piotr-kubiak closed 1 year ago

piotr-kubiak commented 1 year ago

This allows opening multiple certificates in order to compare them.

kaikramer commented 1 year ago

Did you check that this has no unwanted side-effects? You can open more dialogs from the certificate details dialog.

piotr-kubiak commented 1 year ago

I did some testing. No more duplicate dialogs can be opened, as those are modal. Only the Certificate details can be duplicated, which should be fine in most cases. image

kaikramer commented 1 year ago

The ASN.1 window is also non-modal. I was asking because back when I changed the ASN.1 window to non-modal I have also changed the certificate details dialog to non-modal, but had to revert it. Unfortunately I don't remember what the problem was (it was many years ago). It might have been something OS-specific or related to one of the less commonly used features of KSE...

piotr-kubiak commented 1 year ago

OK. I did some more testing and ASN.1 dialog seems fine from all of the opened windows. I also blamed the line back to the first commit and didn't notice it was changed, but of course there may be some edge-case that we are missing. If you're not comfortable with merging, I can always use my fork, it is more than enough for me.

kaikramer commented 1 year ago

I have no doubt that you did everything to make sure this change is working well (there was never a public commit with this, so couldn't know about it being reverted) and I also believe that this is a feature that might be useful for a greater number of users. At the moment I am preparing the next release of KSE and the feature set for this release won't change anymore anyway. So I will keep this PR open and take a closer look at it after KSE 5.5.2 has been released. Then I have time to do some testing etc.

Thank you for the PR!

kaikramer commented 1 year ago

I merge it now. There should be enough time to test it until the next release.

Thank you for your contribution!