# HG changeset patch # User Elliott Baron # Date 1544126172 18000 # Thu Dec 06 14:56:12 2018 -0500 # Node ID e2ae75db9b2f4ce7486b85a26ca5466dee223d02 # Parent 33bb1796210b045da81c2960538e944f7f1ea10b JMC-6180: Changing the Java source editor font changes the size of some values in the JMC tables Summary: Editor font usage in tables/trees should use a font height consistent with other columns Reviewed-by: Contributed-by: Elliott Baron diff --git a/application/org.openjdk.jmc.rjmx.ui/src/main/java/org/openjdk/jmc/rjmx/ui/attributes/ValueColumnLabelProvider.java b/application/org.openjdk.jmc.rjmx.ui/src/main/java/org/openjdk/jmc/rjmx/ui/attributes/ValueColumnLabelProvider.java --- a/application/org.openjdk.jmc.rjmx.ui/src/main/java/org/openjdk/jmc/rjmx/ui/attributes/ValueColumnLabelProvider.java +++ b/application/org.openjdk.jmc.rjmx.ui/src/main/java/org/openjdk/jmc/rjmx/ui/attributes/ValueColumnLabelProvider.java @@ -33,6 +33,8 @@ package org.openjdk.jmc.rjmx.ui.attributes; import org.eclipse.jface.preference.JFacePreferences; +import org.eclipse.jface.resource.FontDescriptor; +import org.eclipse.jface.resource.FontRegistry; import org.eclipse.jface.resource.JFaceResources; import org.eclipse.swt.graphics.Color; import org.eclipse.swt.graphics.Font; @@ -43,6 +45,8 @@ public class ValueColumnLabelProvider extends TypedLabelProvider { + private static final String FIXED_TEXT_FONT = "org.openjdk.jmc.fixedtextfont"; //$NON-NLS-1$ + public ValueColumnLabelProvider() { super(IReadOnlyAttribute.class); } @@ -54,10 +58,23 @@ @Override protected Font getFontTyped(IReadOnlyAttribute attribute) { + if (!JFaceResources.getFontRegistry().hasValueFor(FIXED_TEXT_FONT)) { + createFixedSizeTextFont(); + } if (isValid(attribute)) { - return JFaceResources.getFontRegistry().get(JFaceResources.TEXT_FONT); + return JFaceResources.getFontRegistry().get(FIXED_TEXT_FONT); } - return JFaceResources.getFontRegistry().getItalic(JFaceResources.TEXT_FONT); + return JFaceResources.getFontRegistry().getItalic(FIXED_TEXT_FONT); + } + + private void createFixedSizeTextFont() { + FontRegistry registry = JFaceResources.getFontRegistry(); + // Take the height of the first FontData. In practice, there should only be one. + int defaultHeight = registry.get(JFaceResources.DEFAULT_FONT).getFontData()[0].getHeight(); + + // Modify the TEXT_FONT to have the same height as the DEFAULT_FONT. + FontDescriptor textFontDesc = registry.getDescriptor(JFaceResources.TEXT_FONT).setHeight(defaultHeight); + registry.put(FIXED_TEXT_FONT, textFontDesc.getFontData()); } @Override diff --git a/application/uitests/org.openjdk.jmc.console.uitest/src/test/java/org/openjdk/jmc/console/uitest/MBeanBrowserTabTest.java b/application/uitests/org.openjdk.jmc.console.uitest/src/test/java/org/openjdk/jmc/console/uitest/MBeanBrowserTabTest.java --- a/application/uitests/org.openjdk.jmc.console.uitest/src/test/java/org/openjdk/jmc/console/uitest/MBeanBrowserTabTest.java +++ b/application/uitests/org.openjdk.jmc.console.uitest/src/test/java/org/openjdk/jmc/console/uitest/MBeanBrowserTabTest.java @@ -40,6 +40,10 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.eclipse.jface.resource.FontDescriptor; +import org.eclipse.jface.resource.JFaceResources; +import org.eclipse.swt.graphics.Font; +import org.eclipse.swt.graphics.FontData; import org.junit.Assert; import org.junit.Assume; import org.junit.ClassRule; @@ -51,13 +55,14 @@ import org.openjdk.jmc.test.jemmy.MCUITestRule; import org.openjdk.jmc.test.jemmy.misc.base.wrappers.MCJemmyBase; import org.openjdk.jmc.test.jemmy.misc.helpers.ConnectionHelper; +import org.openjdk.jmc.test.jemmy.misc.wrappers.JmxConsole; import org.openjdk.jmc.test.jemmy.misc.wrappers.MC; import org.openjdk.jmc.test.jemmy.misc.wrappers.MCButton; import org.openjdk.jmc.test.jemmy.misc.wrappers.MCTabFolder; import org.openjdk.jmc.test.jemmy.misc.wrappers.MCTable; import org.openjdk.jmc.test.jemmy.misc.wrappers.MCToolBar; import org.openjdk.jmc.test.jemmy.misc.wrappers.MCTree; -import org.openjdk.jmc.test.jemmy.misc.wrappers.JmxConsole; +import org.openjdk.jmc.ui.misc.DisplayToolkit; /** * Class for for testing MBean Browser related actions in the JMX Console. @@ -77,6 +82,9 @@ private static final String RESULT_TREE_NAME = org.openjdk.jmc.rjmx.ui.operations.ExecuteOperationForm.RESULT_TREE_NAME;; private static final String RESULT_TAB_NAME = org.openjdk.jmc.rjmx.ui.operations.ExecuteOperationForm.RESULT_TAB_NAME; private static final String MBEANBROWSER_NOTIFICATIONSTAB_LOGTREE_NAME = org.openjdk.jmc.console.ui.mbeanbrowser.notifications.MBeanNotificationLogInspector.MBEANBROWSER_NOTIFICATIONSTAB_LOGTREE_NAME; + private static final String VALUE_COLUMN_NAME = org.openjdk.jmc.rjmx.ui.attributes.Messages.AttributeInspector_VALUE_COLUMN_HEADER; + private static final int DEFAULT_FONT_HEIGHT = 11; + private static final int TEXT_FONT_HEIGHT = 16; @Rule public MCUITestRule testRule = new MCUITestRule(verboseRuleOutput) { @@ -97,8 +105,24 @@ @ClassRule public static MCUITestRule classTestRule = new MCUITestRule(verboseRuleOutput) { + + private FontData[] saveDefaultFont; + private FontData[] saveTextFont; + @Override public void before() { + // Set default and text font to some predefined, but different, sizes + DisplayToolkit.safeSyncExec(() -> { + FontData[] defaultFontData = JFaceResources.getDefaultFont().getFontData(); + saveDefaultFont = FontDescriptor.copy(defaultFontData); + defaultFontData[0].setHeight(DEFAULT_FONT_HEIGHT); + JFaceResources.getFontRegistry().put(JFaceResources.DEFAULT_FONT, defaultFontData); + + FontData[] textFontData = JFaceResources.getTextFont().getFontData(); + saveTextFont = FontDescriptor.copy(textFontData); + textFontData[0].setHeight(TEXT_FONT_HEIGHT); + JFaceResources.getFontRegistry().put(JFaceResources.TEXT_FONT, textFontData); + }); // not using the default test connection since we're starting threads of interest in this particular JVM MC.jvmBrowser.connect("The JVM Running Mission Control"); initialiseTestThreads(); @@ -107,6 +131,15 @@ @Override public void after() { terminateTestThreads(); + // Restore original font sizes + DisplayToolkit.safeSyncExec(() -> { + if (saveDefaultFont != null) { + JFaceResources.getFontRegistry().put(JFaceResources.DEFAULT_FONT, saveDefaultFont); + } + if (saveTextFont != null) { + JFaceResources.getFontRegistry().put(JFaceResources.TEXT_FONT, saveTextFont); + } + }); } }; @@ -332,6 +365,52 @@ Assert.assertTrue("-1", patternMatcher(resultFolder.getText(), "-1")); } + /** + * Verify that the Mbean Browser page Operations works as expected + */ + @Test + public void testValueFontSize() { + // Select the Threading MBean + MCTree.getByName(MBEANBROWSER_TREE_NAME).select("java.lang", "Threading"); + + // Select the attributes tab + MCTabFolder.getByTabName(ATTRIBUTES_TAB).select(ATTRIBUTES_TAB); + + // Finding the table that contains an item that matches the command we want to perform + MCTable operationsTable = getOperationsTable(THREAD_INFO_COMMAND); + + MCTree paramsTree = null; + + // Get the indexes of the matching commands + List indexes = operationsTable.getItemIndexes(THREAD_INFO_COMMAND); + + // For each of the indexes select each item until we find the one we need + for (int i : indexes) { + operationsTable.select(i); + MCTree thisTree = MCTree.getByItem("p0"); + if (!thisTree.hasItem("p1")) { + paramsTree = thisTree; + break; + } + } + // Make sure that we actually found a matching command + Assert.assertNotNull("Could not find the parameter tree", paramsTree); + + // Get the font used by the Value column in the tree + int valueColIdx = paramsTree.getColumnIndex(VALUE_COLUMN_NAME); + paramsTree.select("p0"); + List fonts = paramsTree.getSelectedItemFonts(); + Font valueFont = fonts.get(valueColIdx); + + // Ensure that the font is the text font, but sized to match the default font + final Font[] textFontHolder = new Font[1]; + DisplayToolkit.safeSyncExec(() -> { + textFontHolder[0] = JFaceResources.getFontRegistry().getItalic(JFaceResources.TEXT_FONT); + }); + FontData[] expectedFontData = FontDescriptor.createFrom(textFontHolder[0]).setHeight(DEFAULT_FONT_HEIGHT).getFontData(); + Assert.assertArrayEquals(expectedFontData, valueFont.getFontData()); + } + private List getLatestNotificationLogEntry(MCTree logTree) { List> log = logTree.getAllItemTexts(); if (log.size() == 0) { diff --git a/application/uitests/org.openjdk.jmc.test.jemmy/src/test/java/org/openjdk/jmc/test/jemmy/misc/wrappers/MCTree.java b/application/uitests/org.openjdk.jmc.test.jemmy/src/test/java/org/openjdk/jmc/test/jemmy/misc/wrappers/MCTree.java --- a/application/uitests/org.openjdk.jmc.test.jemmy/src/test/java/org/openjdk/jmc/test/jemmy/misc/wrappers/MCTree.java +++ b/application/uitests/org.openjdk.jmc.test.jemmy/src/test/java/org/openjdk/jmc/test/jemmy/misc/wrappers/MCTree.java @@ -37,6 +37,7 @@ import java.util.List; import java.util.stream.Collectors; +import org.eclipse.swt.graphics.Font; import org.eclipse.swt.graphics.Image; import org.eclipse.swt.graphics.Rectangle; import org.eclipse.swt.widgets.Display; @@ -404,6 +405,33 @@ } /** + * Returns a list of the currently selected tree item's fonts, ordered + * by column + * + * @return a {@link List} of {@link Font} + */ + public List getSelectedItemFonts() { + Fetcher> fetcher = new Fetcher>() { + @Override + public void run() { + List fonts = new ArrayList<>(); + int columnCount = getColumnCount(); + TreeItem selectedItem = control.as(TreeWrap.class).getSelectedItem(); + if (columnCount > 0) { + for (int i = 0; i < columnCount; i++) { + fonts.add(selectedItem.getFont(i)); + } + } else { + fonts.add(selectedItem.getFont()); + } + setOutput(fonts); + } + }; + Display.getDefault().syncExec(fetcher); + return fetcher.getOutput(); + } + + /** * Get the currently selected item's direct child item texts * * @return a {@link List} of {@link String}