Subject: [PATCH] #11153: improve readability of validator warnings --- Index: src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java b/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java --- a/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java (revision 18627) +++ b/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java (date 1673374027806) @@ -11,6 +11,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -44,6 +45,7 @@ import org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.MapCSSParser; import org.openstreetmap.josm.gui.mappaint.mapcss.parsergen.ParseException; import org.openstreetmap.josm.io.IllegalDataException; +import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.tools.CheckParameterUtil; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; @@ -277,7 +279,19 @@ Integer.parseInt(m.group(1)), m.group(2), p); try { // Perform replacement with null-safe + regex-safe handling - m.appendReplacement(sb, String.valueOf(argument).replace("^(", "").replace(")$", "")); + final String replacement = String.valueOf(argument).replace("^(", "").replace(")$", ""); + final String type = m.group(2); + final String url; + if ("key".equals(type) || "tag".equals(type)) { + url = Config.getUrls().getOSMWiki() + "/wiki/" + + m.group(2).substring(0, 1).toUpperCase(Locale.ROOT) + m.group(2).substring(1) + + ":" + replacement; + } else { + url = null; + } + m.appendReplacement(sb, (url != null ? "" : "") + + replacement + + (url != null ? "" : "")); } catch (IndexOutOfBoundsException | IllegalArgumentException e) { Logging.log(Logging.LEVEL_ERROR, tr("Unable to replace argument {0} in {1}: {2}", argument, sb, e.getMessage()), e); } Index: src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanel.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanel.java b/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanel.java --- a/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanel.java (revision 18627) +++ b/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreePanel.java (date 1673371973260) @@ -267,7 +267,7 @@ } final String msg = addSize(searchMsg, errorsWithDescription); - final DefaultMutableTreeNode messageNode = new DefaultMutableTreeNode(msg); + final DefaultMutableTreeNode messageNode = new DefaultMutableTreeNode("" + msg + ""); DefaultMutableTreeNode currNode = groupNode != null ? groupNode : severityNode; currNode.add(messageNode); if (oldExpandedRows.contains(searchMsg)) { Index: src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreeRenderer.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreeRenderer.java b/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreeRenderer.java --- a/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreeRenderer.java (revision 18627) +++ b/src/org/openstreetmap/josm/gui/dialogs/validator/ValidatorTreeRenderer.java (date 1673368470256) @@ -3,39 +3,45 @@ import java.awt.Component; +import javax.swing.JLabel; import javax.swing.JTree; import javax.swing.tree.DefaultMutableTreeNode; -import javax.swing.tree.DefaultTreeCellRenderer; import org.openstreetmap.josm.data.validation.Severity; import org.openstreetmap.josm.data.validation.TestError; import org.openstreetmap.josm.data.validation.util.MultipleNameVisitor; +import org.openstreetmap.josm.gui.widgets.HtmlTreeCellRenderer; import org.openstreetmap.josm.tools.ImageProvider; /** * Tree renderer for displaying errors * @author frsantos */ -public class ValidatorTreeRenderer extends DefaultTreeCellRenderer { +public class ValidatorTreeRenderer extends HtmlTreeCellRenderer { + + private static final long serialVersionUID = 4750085115702320153L; @Override public Component getTreeCellRendererComponent(JTree tree, Object value, boolean selected, boolean expanded, boolean leaf, int row, boolean hasFocus) { - super.getTreeCellRendererComponent(tree, value, selected, expanded, leaf, row, hasFocus); + final Component component = super.getTreeCellRendererComponent(tree, value, selected, expanded, leaf, row, hasFocus); - DefaultMutableTreeNode node = (DefaultMutableTreeNode) value; - Object nodeInfo = node.getUserObject(); + final DefaultMutableTreeNode node = (DefaultMutableTreeNode) value; + final Object nodeInfo = node.getUserObject(); - if (nodeInfo instanceof Severity) { - Severity s = (Severity) nodeInfo; - setIcon(ImageProvider.get("data", s.getIcon())); - } else if (nodeInfo instanceof TestError) { - TestError error = (TestError) nodeInfo; - MultipleNameVisitor v = error.getNameVisitor(); - setText(v.getText()); - setIcon(v.getIcon()); + if (component instanceof JLabel) { + final JLabel label = (JLabel) component; + if (nodeInfo instanceof Severity) { + final Severity s = (Severity) nodeInfo; + label.setIcon(ImageProvider.get("data", s.getIcon())); + } else if (nodeInfo instanceof TestError) { + final TestError error = (TestError) nodeInfo; + MultipleNameVisitor v = error.getNameVisitor(); + label.setText(v.getText()); + label.setIcon(v.getIcon()); + } } - return this; + return component; } } Index: src/org/openstreetmap/josm/gui/widgets/HtmlTreeCellRenderer.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/src/org/openstreetmap/josm/gui/widgets/HtmlTreeCellRenderer.java b/src/org/openstreetmap/josm/gui/widgets/HtmlTreeCellRenderer.java new file mode 100644 --- /dev/null (date 1673387854307) +++ b/src/org/openstreetmap/josm/gui/widgets/HtmlTreeCellRenderer.java (date 1673387854307) @@ -0,0 +1,199 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.gui.widgets; + +import java.awt.Color; +import java.awt.Component; +import java.awt.Point; +import java.awt.Rectangle; +import java.awt.event.MouseEvent; +import java.text.MessageFormat; +import java.util.Arrays; + +import javax.annotation.Nullable; +import javax.swing.JTree; +import javax.swing.event.HyperlinkEvent; +import javax.swing.event.MouseInputAdapter; +import javax.swing.text.html.HTMLDocument; +import javax.swing.text.html.StyleSheet; +import javax.swing.tree.DefaultMutableTreeNode; +import javax.swing.tree.DefaultTreeCellRenderer; +import javax.swing.tree.TreePath; + +import org.openstreetmap.josm.tools.ColorHelper; +import org.openstreetmap.josm.tools.OpenBrowser; + +/** + * A {@link javax.swing.tree.TreeCellRenderer} for cells that may contain hyperlinks. + * @since xxx + */ +public class HtmlTreeCellRenderer extends DefaultTreeCellRenderer { + /** + * This only exists since the JTree cannot pass events to subcomponents. + */ + private static class JTreeMouseInputListener extends MouseInputAdapter { + @Override + public void mouseClicked(MouseEvent mouseEvent) { + if (mouseEvent.getComponent() instanceof JTree) { + final Point p = mouseEvent.getPoint(); + final JTree tree = (JTree) mouseEvent.getComponent(); + final Component component = getComponentAt(tree, p); // p is translated here + if (component != null) { + component.dispatchEvent(new MouseEvent(component, // Use the component we found to fire the event + mouseEvent.getID(), mouseEvent.getWhen(), mouseEvent.getModifiers(), + p.x, p.y, mouseEvent.getClickCount(), mouseEvent.isPopupTrigger(), + mouseEvent.getButton())); + } + } + } + + /** + * Get a component at + * @param tree The tree to get the component from + * @param p The point to get the component at (will be modified) + * @return The component + */ + @Nullable + private static Component getComponentAt(JTree tree, Point p) { + final TreePath path = tree.getPathForLocation(p.x, p.y); + if (path != null) { + final int row = tree.getRowForPath(path); + final Object last = path.getLastPathComponent(); + // We need to get the component as "shown" so that we can get the link clicked. + final Component component = tree.getCellRenderer().getTreeCellRendererComponent(tree, last, + tree.isRowSelected(row), tree.isExpanded(row), + tree.getModel().isLeaf(last), row, true); + final Rectangle bounds = tree.getPathBounds(path); + if (bounds != null) { + // If we don't translate the point, we are attempting to get the link in the wrong frame of reference + // The reference x/y are 0. This is set in BasicTextUI#getVisibleEditorRect, so we must translate the point here. + p.translate(-bounds.x, -bounds.y); + // Just make certain that we are consistent for other objects. + bounds.x = 0; + bounds.y = 0; + // Set the bounds from the JTree component (needed for proper layout) + component.setBounds(bounds); + return component; + } + } + return null; + } + } + + private static final long serialVersionUID = -4842755204541968238L; + /** + * A reusable panel to avoid new objects where possible. + * It isn't worth it to make a new object for each row, since it doesn't receive mouse events (tested on Java 8). + */ + private final JosmEditorPane panel = new JosmEditorPane(); + /** JTree does not send mouse events to subcomponents */ + private final JTreeMouseInputListener mouseTreeListener = new JTreeMouseInputListener(); + + /** + * Create a new renderer + */ + public HtmlTreeCellRenderer() { + panel.addHyperlinkListener(e -> { + if (e.getURL() != null && HyperlinkEvent.EventType.ACTIVATED.equals(e.getEventType())) { + OpenBrowser.displayUrl(e.getURL().toString()); + } + }); + JosmEditorPane.makeJLabelLike(panel, false); + final Color defaultLink = ColorHelper.html2color(JosmEditorPane.getLinkColor()); + final Color lighterLink = defaultLink.brighter().brighter(); + final Color darkerLink = defaultLink.darker().darker(); + final String divRule = "{0} '{'background-color:{1}; color:{2};'}'"; + final String aRule = "{0} a '{'text-decoration: underline; color:{1}'}'"; + StyleSheet ss = ((HTMLDocument) panel.getDocument()).getStyleSheet(); + ss.addRule(MessageFormat.format(divRule, ".selected", ColorHelper.color2html(getBackgroundSelectionColor()), + ColorHelper.color2html(getTextSelectionColor()))); + ss.addRule(MessageFormat.format(aRule, ".selected", + ColorHelper.color2html(getAppropriateColor(getBackgroundSelectionColor(), defaultLink, lighterLink, darkerLink)))); + ss.addRule(MessageFormat.format(divRule, ".not-selected", ColorHelper.color2html(getBackgroundNonSelectionColor()), + ColorHelper.color2html(getTextNonSelectionColor()))); + ss.addRule(MessageFormat.format(aRule, ".not-selected", + ColorHelper.color2html(getAppropriateColor(getBackgroundNonSelectionColor(), defaultLink, lighterLink, darkerLink)))); + } + + @Override + public Component getTreeCellRendererComponent(JTree tree, Object value, boolean sel, boolean expanded, + boolean leaf, int row, boolean hasFocus) { + if (value instanceof DefaultMutableTreeNode) { + if (Arrays.stream(tree.getMouseListeners()).noneMatch(this.mouseTreeListener::equals)) { + tree.addMouseListener(this.mouseTreeListener); + tree.addMouseMotionListener(this.mouseTreeListener); + } + final DefaultMutableTreeNode node = (DefaultMutableTreeNode) value; + final Object object = node.getUserObject(); + if (object instanceof String && ((String) object).startsWith("") && ((String) object).endsWith("")) { + final String text = (String) object; + final String modified = "
" + + text.substring("".length(), text.length() - "".length()) + "
"; + this.panel.setText(modified); + return this.panel; + } + } + return super.getTreeCellRendererComponent(tree, value, sel, expanded, leaf, row, hasFocus); + } + + /** + * Convert the rgb value for use in a relative luminance calculation + * @param rgb The value to convert + * @return The appropriate value to use in the calculation + */ + private static double convertRGB(int rgb) { + final double normalized = rgb / 255d; + if (normalized <= 0.03928) { + return normalized / 12.92; + } + return Math.pow(((normalized + 0.055)/1.055), 2.4); + } + + /** + * Calculate the contrast ratio between two luminance values + * @param luminanceOne The first luminance value + * @param luminanceTwo The second luminance value + * @return The contrast ratio. Higher is usually better. + */ + private static double contrastRatio(double luminanceOne, double luminanceTwo) { + final double min = Math.min(luminanceOne, luminanceTwo); + final double max = Math.max(luminanceOne, luminanceTwo); + return (max + 0.05) / (min + 0.05); + } + + /** + * Calculate the luminance of a color + * @param red The red part + * @param green The green part + * @param blue The blue part + * @return The relative luminance (0-1, 0 black, 1 white) + */ + private static double luminance(int red, int green, int blue) { + return 0.2126 * convertRGB(red) + + 0.7152 * convertRGB(green) + + 0.0722 * convertRGB(blue); + } + + /** + * Get the appropriate color given a background + * @param background The background color + * @param options The options to choose from + * @return The appropriate color + */ + private static Color getAppropriateColor(Color background, Color... options) { + if (options.length < 2) { + throw new IllegalArgumentException("There must be at least two color options"); + } + // sRGB calculation + final double backgroundLuminance = luminance(background.getRed(), background.getGreen(), background.getBlue()); + double lastContrastRatio = 0; // W3 recommends a contrast ratio of at least 4.5:1. We should be shooting for 7:1. + Color lastColor = null; + for (Color option : options) { + final double contrastRatio = contrastRatio(backgroundLuminance, luminance(option.getRed(), option.getGreen(), option.getBlue())); + if (contrastRatio > lastContrastRatio) { + lastContrastRatio = contrastRatio; + lastColor = option; + } + } + return lastColor; + } +}