Bug 578024 - Improve the TrustCertificateDialog

Use a SashForm so that relative sizes of the top table and the bottom
tree can be controlled by the user.

Use a wrapping label for the description text such that a longer
description doesn't make the dialog too wide.

Move the warning text about the name not being reliable from the column
label to the description.

Use a TableColumnLayout so that all the columns are visible without
scrolling.

Fix the problem with the false indication of expiration, i.e., when the
valid seconds is 0, that means there is no expiration.

Change-Id: Ib3f41145174a066d0c48c4e1cc91ca580f89b510
Signed-off-by: Ed Merks <ed.merks@gmail.com>
Reviewed-on: https://git.eclipse.org/r/c/equinox/rt.equinox.p2/+/190485
diff --git a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/dialogs/TrustCertificateDialog.java b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/dialogs/TrustCertificateDialog.java
index 6ef4bec..a5d0d07 100644
--- a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/dialogs/TrustCertificateDialog.java
+++ b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/dialogs/TrustCertificateDialog.java
@@ -30,11 +30,16 @@
 import org.eclipse.equinox.internal.p2.ui.viewers.CertificateLabelProvider;
 import org.eclipse.equinox.internal.provisional.security.ui.X500PrincipalHelper;
 import org.eclipse.equinox.internal.provisional.security.ui.X509CertificateViewDialog;
+import org.eclipse.equinox.p2.repository.spi.PGPPublicKeyService;
 import org.eclipse.jface.dialogs.Dialog;
 import org.eclipse.jface.dialogs.IDialogConstants;
+import org.eclipse.jface.layout.TableColumnLayout;
 import org.eclipse.jface.viewers.*;
+import org.eclipse.jface.widgets.LabelFactory;
+import org.eclipse.jface.widgets.WidgetFactory;
 import org.eclipse.osgi.util.NLS;
 import org.eclipse.swt.SWT;
+import org.eclipse.swt.custom.SashForm;
 import org.eclipse.swt.dnd.*;
 import org.eclipse.swt.events.SelectionEvent;
 import org.eclipse.swt.events.SelectionListener;
@@ -52,9 +57,6 @@
 	private Object inputElement;
 	private IStructuredContentProvider contentProvider;
 
-	private static final int SIZING_SELECTION_WIDGET_HEIGHT = 250;
-	private static final int SIZING_SELECTION_WIDGET_WIDTH = 300;
-
 	CheckboxTableViewer listViewer;
 
 	private TreeViewer certificateChainViewer;
@@ -124,12 +126,14 @@
 
 	@Override
 	protected Control createDialogArea(Composite parent) {
-		Composite composite = createUpperDialogArea(parent);
+		SashForm sashForm = createUpperDialogArea(parent);
+		Composite composite = createSashFormArea(sashForm);
+
 		certificateChainViewer = new TreeViewer(composite, SWT.BORDER);
-		GridLayout layout = new GridLayout();
-		certificateChainViewer.getTree().setLayout(layout);
 		GridData data = new GridData(GridData.FILL_BOTH);
 		data.grabExcessHorizontalSpace = true;
+		data.heightHint = convertHeightInCharsToPixels(8);
+		data.widthHint = convertWidthInCharsToPixels(120);
 		certificateChainViewer.getTree().setLayoutData(data);
 		certificateChainViewer.setAutoExpandLevel(AbstractTreeViewer.ALL_LEVELS);
 		certificateChainViewer.setContentProvider(new TreeNodeContentProvider());
@@ -143,7 +147,7 @@
 						String userFriendlyFingerPrint = userFriendlyFingerPrint(key);
 						java.util.List<String> users = new ArrayList<>();
 						key.getUserIDs().forEachRemaining(users::add);
-						String userIDs = String.join(",", users); //$NON-NLS-1$
+						String userIDs = String.join(", ", users); //$NON-NLS-1$
 						if (!userIDs.isEmpty()) {
 							return userFriendlyFingerPrint + " [" + userIDs + "]"; //$NON-NLS-1$//$NON-NLS-2$
 						}
@@ -175,16 +179,17 @@
 
 		listViewer.addDoubleClickListener(getDoubleClickListener());
 		listViewer.addSelectionChangedListener(getParentSelectionListener());
+
 		createButtons(composite);
+
 		if (inputElement instanceof Object[]) {
-			ISelection selection = null;
 			Object[] nodes = (Object[]) inputElement;
 			if (nodes.length > 0) {
-				selection = new StructuredSelection(nodes[0]);
+				ISelection selection = new StructuredSelection(nodes[0]);
 				certificateChainViewer.setInput(new TreeNode[] { (TreeNode) nodes[0] });
 				certificateChainViewer.setSelection(selection);
+				listViewer.setSelection(selection);
 			}
-			listViewer.setSelection(selection);
 		}
 		return composite;
 	}
@@ -275,19 +280,52 @@
 		});
 	}
 
-	private Composite createUpperDialogArea(Composite parent) {
-		Composite composite = (Composite) super.createDialogArea(parent);
-		initializeDialogUnits(composite);
+	private Composite createSashFormArea(SashForm sashForm) {
+		Composite composite = new Composite(sashForm, SWT.NONE);
+		GridLayout layout = new GridLayout();
+		layout.marginHeight = 0;
+		layout.marginWidth = 0;
+		layout.verticalSpacing = convertVerticalDLUsToPixels(IDialogConstants.VERTICAL_SPACING);
+		layout.horizontalSpacing = convertHorizontalDLUsToPixels(IDialogConstants.HORIZONTAL_SPACING);
+		composite.setLayout(layout);
+		return composite;
+	}
+
+	@Override
+	protected Label createMessageArea(Composite composite) {
+		GridData data = new GridData(SWT.FILL, SWT.FILL, true, false);
+		data.widthHint = convertWidthInCharsToPixels(120);
+		LabelFactory factory = WidgetFactory.label(SWT.WRAP).font(composite.getFont()).layoutData(data);
+		if (getMessage() != null) {
+			factory.text(getMessage());
+		}
+		return factory.create(composite);
+	}
+
+	private SashForm createUpperDialogArea(Composite parent) {
+		Composite mainComposite = (Composite) super.createDialogArea(parent);
+		initializeDialogUnits(mainComposite);
+
+		SashForm sashForm = new SashForm(mainComposite, SWT.VERTICAL);
+		sashForm.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));
+		Composite composite = createSashFormArea(sashForm);
+
 		createMessageArea(composite);
 
-		listViewer = CheckboxTableViewer.newCheckList(composite, SWT.BORDER | SWT.FULL_SELECTION);
-		GridData data = new GridData(GridData.FILL_BOTH);
-		data.heightHint = SIZING_SELECTION_WIDGET_HEIGHT;
-		data.widthHint = SIZING_SELECTION_WIDGET_WIDTH;
-		listViewer.getTable().setLayoutData(data);
-		listViewer.getTable().setHeaderVisible(true);
-
+		TableColumnLayout tableColumnLayout = new TableColumnLayout();
+		Composite tableComposite = WidgetFactory.composite(SWT.NONE)
+				.layoutData(new GridData(SWT.FILL, SWT.FILL, true, true)).layout(tableColumnLayout).create(composite);
+		Table table = WidgetFactory
+				.table(SWT.H_SCROLL | SWT.V_SCROLL | SWT.MULTI | SWT.BORDER | SWT.FULL_SELECTION | SWT.CHECK)
+				.headerVisible(true).linesVisible(true).font(parent.getFont()).create(tableComposite);
+		listViewer = new CheckboxTableViewer(table);
 		listViewer.setContentProvider(contentProvider);
+
+		GridData data = new GridData(GridData.FILL_BOTH);
+		data.heightHint = convertHeightInCharsToPixels(10);
+		data.widthHint = convertWidthInCharsToPixels(120);
+		tableComposite.setLayoutData(data);
+
 		TableViewerColumn typeColumn = new TableViewerColumn(listViewer, SWT.NONE);
 		typeColumn.getColumn().setText(ProvUIMessages.TrustCertificateDialog_ObjectType);
 		typeColumn.setLabelProvider(new PGPOrX509ColumnLabelProvider(key -> "PGP", cert -> "x509")); //$NON-NLS-1$ //$NON-NLS-2$
@@ -302,7 +340,7 @@
 		signerColumn.setLabelProvider(new PGPOrX509ColumnLabelProvider(pgp -> {
 			java.util.List<String> users = new ArrayList<>();
 			pgp.getUserIDs().forEachRemaining(users::add);
-			return String.join(",", users); //$NON-NLS-1$
+			return String.join(", ", users); //$NON-NLS-1$
 		}, x509 -> {
 			X500PrincipalHelper principalHelper = new X500PrincipalHelper(x509.getSubjectX500Principal());
 			return principalHelper.getCN() + "; " + principalHelper.getOU() + "; " //$NON-NLS-1$ //$NON-NLS-2$
@@ -315,7 +353,11 @@
 			if (pgp.getCreationTime().after(Date.from(Instant.now()))) {
 				return NLS.bind(ProvUIMessages.TrustCertificateDialog_NotYetValidStartDate, pgp.getCreationTime());
 			}
-			Instant expires = pgp.getCreationTime().toInstant().plus(pgp.getValidSeconds(), ChronoUnit.SECONDS);
+			long validSeconds = pgp.getValidSeconds();
+			if (validSeconds == 0) {
+				return ProvUIMessages.TrustCertificateDialog_valid;
+			}
+			Instant expires = pgp.getCreationTime().toInstant().plus(validSeconds, ChronoUnit.SECONDS);
 			return expires.isBefore(Instant.now())
 					? NLS.bind(ProvUIMessages.TrustCertificateDialog_expiredSince, expires)
 					: NLS.bind(ProvUIMessages.TrustCertificateDialog_validExpires, expires);
@@ -330,8 +372,14 @@
 			}
 		}));
 
-		Menu menu = new Menu(listViewer.getTable());
-		listViewer.getTable().setMenu(menu);
+		// The first column is packed below.
+		tableColumnLayout.setColumnData(typeColumn.getColumn(), new ColumnWeightData(1));
+		tableColumnLayout.setColumnData(idColumn.getColumn(), new ColumnWeightData(10));
+		tableColumnLayout.setColumnData(signerColumn.getColumn(), new ColumnWeightData(15));
+		tableColumnLayout.setColumnData(validColumn.getColumn(), new ColumnWeightData(10));
+
+		Menu menu = new Menu(table);
+		table.setMenu(menu);
 		MenuItem item = new MenuItem(menu, SWT.PUSH);
 		item.setText(ProvUIMessages.TrustCertificateDialog_CopyFingerprint);
 		item.addSelectionListener(widgetSelectedAdapter(e -> {
@@ -354,9 +402,6 @@
 		listViewer.setInput(inputElement);
 
 		typeColumn.getColumn().pack();
-		idColumn.getColumn().pack();
-		signerColumn.getColumn().pack();
-		validColumn.getColumn().pack();
 
 		if (!getInitialElementSelections().isEmpty()) {
 			checkInitialSelections();
@@ -364,7 +409,7 @@
 
 		Dialog.applyDialogFont(composite);
 
-		return composite;
+		return sashForm;
 	}
 
 	/**
@@ -484,10 +529,6 @@
 		if (key == null) {
 			return null;
 		}
-		StringBuilder builder = new StringBuilder();
-		for (byte b : key.getFingerprint()) {
-			builder.append(String.format("%02X", Byte.toUnsignedInt(b))); //$NON-NLS-1$
-		}
-		return builder.toString();
+		return PGPPublicKeyService.toHex(key.getFingerprint()).toUpperCase(Locale.ROOT);
 	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/messages.properties b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/messages.properties
index 8b0a903..af618c8 100644
--- a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/messages.properties
+++ b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/messages.properties
@@ -280,14 +280,15 @@
 TrustCertificateDialog_Export=\uD83D\uDCE5 E&xport...
 TrustCertificateDialog_Title=Trust
 TrustCertificateDialog_Message=Do you trust these signers?
-TrustCertificateDialog_MessageWithPGP=Do you trust these signers?\n\
+TrustCertificateDialog_MessageWithPGP=Do you trust these signers?  \
+\u26A0\uFE0F\u00A0The displayed originator names are not necessarily a reliable certification or origin.  \
 For PGP keys, verification is typically achieved by querying the key\'s fingerprint against a trusted key server.
 TrustCertificateDialog_AcceptSelectedButtonLabel=&Trust Selected
 TrustCertificateDialog_SelectAll=&Select All
 TrustCertificateDialog_DeselectAll=&Deselect All
 TrustCertificateDialog_ObjectType=Type
 TrustCertificateDialog_Id=Id/Fingerprint
-TrustCertificateDialog_Name=Name \u26A0\uFE0F This is not trustworthy until key/certificate is verified
+TrustCertificateDialog_Name=Name
 TrustCertificateDialog_CopyFingerprint=Copy Fingerprint
 TrustCertificateDialog_dates=Validity Dates
 TrustCertificateDialog_NotYetValidStartDate=\u274C Not yet valid, starts {0}