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 18633)
+++ b/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java	(date 1673548870696)
@@ -270,14 +270,30 @@
         } else if (s == null || !(matchingSelector instanceof Selector.GeneralSelector)) {
             return s;
         }
-        final Matcher m = Pattern.compile("\\{(\\d+)\\.(key|value|tag)\\}").matcher(s);
+        final Matcher m = Pattern.compile("\\{(\\S+?)\\.(key|value|tag)}", Pattern.UNICODE_CHARACTER_CLASS).matcher(s);
+        final Matcher number = Pattern.compile("^\\d+$").matcher(s);
         final StringBuffer sb = new StringBuffer();
         while (m.find()) {
-            final String argument = determineArgument((Selector.GeneralSelector) matchingSelector,
-                    Integer.parseInt(m.group(1)), m.group(2), p);
+            final String argument;
+            final String tag;
+            if (number.reset(m.group(1)).matches()) {
+                argument = determineArgument((Selector.GeneralSelector) matchingSelector,
+                        Integer.parseInt(m.group(1)), m.group(2), p);
+                tag = determineArgument((Selector.GeneralSelector) matchingSelector,
+                        Integer.parseInt(m.group(1)), "tag", p);
+            } else {
+                argument = m.group(1);
+                tag = argument;
+            }
             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 replacementTag = String.valueOf(tag).replace("^(", "").replace(")$", "");
+                final String[] splitTag = replacementTag.split("=", 2);
+                final String url = Test.getWikiLink(splitTag[0], !"key".equals(m.group(2)) && splitTag.length == 2 ? splitTag[1] : null);
+                m.appendReplacement(sb, "<a href=\"" + url + "\">"
+                        + replacement
+                        + "</a>");
             } 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/data/validation/Test.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/org/openstreetmap/josm/data/validation/Test.java b/src/org/openstreetmap/josm/data/validation/Test.java
--- a/src/org/openstreetmap/josm/data/validation/Test.java	(revision 18633)
+++ b/src/org/openstreetmap/josm/data/validation/Test.java	(date 1673547219899)
@@ -11,6 +11,8 @@
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import javax.swing.JCheckBox;
 import javax.swing.JPanel;
 
@@ -26,6 +28,7 @@
 import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
+import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.GBC;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Stopwatch;
@@ -351,7 +354,7 @@
      * @param p The primitive to be tested
      * @return True if building key is set and different from no,entrance
      */
-    protected static final boolean isBuilding(OsmPrimitive p) {
+    protected static boolean isBuilding(OsmPrimitive p) {
         return p.hasTagDifferent("building", "no", "entrance");
     }
 
@@ -360,10 +363,26 @@
      * @param p The primitive to be tested
      * @return True if landuse key is equal to residential
      */
-    protected static final boolean isResidentialArea(OsmPrimitive p) {
+    protected static boolean isResidentialArea(OsmPrimitive p) {
         return p.hasTag("landuse", "residential");
     }
 
+    /**
+     * Get the wiki link for a specified key and value
+     * @param key The key
+     * @param value The value
+     * @return The wiki link
+     * @since xxx
+     */
+    @Nonnull
+    public static String getWikiLink(@Nonnull String key, @Nullable String value) {
+        final String wiki = Config.getUrls().getOSMWiki() + "/wiki/";
+        if (value == null) {
+            return wiki + "Key:" + key;
+        }
+        return wiki + "Tag:" + key + "=" + value;
+    }
+
     /**
      * Free resources.
      */
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 18633)
+++ 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("<html>" + msg + "</html>");
                     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 18633)
+++ 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 1673549280789)
+++ b/src/org/openstreetmap/josm/gui/widgets/HtmlTreeCellRenderer.java	(date 1673549280789)
@@ -0,0 +1,259 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.gui.widgets;
+
+import java.awt.Color;
+import java.awt.Component;
+import java.awt.Cursor;
+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.JLabel;
+import javax.swing.JTree;
+import javax.swing.event.HyperlinkEvent;
+import javax.swing.event.MouseInputListener;
+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 implements MouseInputListener {
+        @Override
+        public void mouseClicked(MouseEvent mouseEvent) {
+            dispatchEvent(mouseEvent);
+        }
+
+        @Override
+        public void mouseMoved(MouseEvent mouseEvent) {
+            dispatchEvent(mouseEvent);
+        }
+
+        @Override
+        public void mousePressed(MouseEvent mouseEvent) {
+            // Do nothing -- this can cause issues with the popup menu
+        }
+
+        @Override
+        public void mouseReleased(MouseEvent mouseEvent) {
+            // Do nothing -- this can cause issues with the popup menu
+        }
+
+        @Override
+        public void mouseEntered(MouseEvent mouseEvent) {
+            dispatchEvent(mouseEvent);
+        }
+
+        @Override
+        public void mouseExited(MouseEvent mouseEvent) {
+            dispatchEvent(mouseEvent);
+        }
+
+        @Override
+        public void mouseDragged(MouseEvent mouseEvent) {
+            dispatchEvent(mouseEvent);
+        }
+
+        /**
+         * Dispatch mouse events for HTML-like cells
+         * @param mouseEvent The event to dispatch
+         */
+        private static void dispatchEvent(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()));
+                    component.setBounds(0, 0, 0, 0);
+                }
+            }
+        }
+
+        /**
+         * Get a rendered component for HTML
+         * @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 && tree.getCellRenderer() instanceof HtmlTreeCellRenderer) {
+                final HtmlTreeCellRenderer renderer = (HtmlTreeCellRenderer) tree.getCellRenderer();
+                final JosmEditorPane panel = renderer.panel;
+                final int row = tree.getRowForPath(path);
+                final Object last = path.getLastPathComponent();
+                // We need to get the right text into the panel, so call the renderer
+                final Component component = renderer.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.
+                    final int width = (component instanceof JLabel)
+                            ? ((JLabel) component).getIcon().getIconWidth() + ((JLabel) component).getIconTextGap()
+                            : 0;
+                    // This translation puts the origin point at the upper-left of the JLabel, but moves it to the right of the icon to the text
+                    // Yes, it is - width, not + width. If you change this, make certain that clicking links still works!
+                    p.translate(-bounds.x - width, -bounds.y);
+                    // Just make certain that we are consistent for other objects.
+                    bounds.x = 0;
+                    bounds.y = 0;
+                    // Set the correct width
+                    bounds.width -= width;
+                    // Set the bounds from the JTree component (needed for proper layout)
+                    panel.setBounds(bounds);
+                    return panel;
+                }
+            }
+            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();
+    /** The tree used to set cursors when entering/leaving a hyperlink */
+    private JTree tree;
+
+    /**
+     * Create a new renderer
+     */
+    public HtmlTreeCellRenderer() {
+        panel.addHyperlinkListener(e -> {
+            if (e.getURL() != null) {
+                if (HyperlinkEvent.EventType.ACTIVATED.equals(e.getEventType())) {
+                    OpenBrowser.displayUrl(e.getURL().toString());
+                } else if (this.tree != null && HyperlinkEvent.EventType.ENTERED.equals(e.getEventType())) {
+                    this.tree.setCursor(Cursor.getPredefinedCursor(Cursor.HAND_CURSOR));
+                } else if (this.tree != null && HyperlinkEvent.EventType.EXITED.equals(e.getEventType())) {
+                    this.tree.setCursor(Cursor.getDefaultCursor());
+                }
+            }
+        });
+        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) {
+        final Component component = super.getTreeCellRendererComponent(tree, value, sel, expanded, leaf, row, hasFocus);
+        this.tree = tree;
+        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("<html>") && ((String) object).endsWith("</html>")) {
+                final String text = (String) object;
+                final String modified = "<html><div class=\"" + (sel ? "selected" : "not-selected") + "\">"
+                        + text.substring("<html>".length(), text.length() - "</html>".length()) + "</div></html>";
+                this.panel.setText(modified);
+                this.setText(this.panel.getText()); // This gets the custom CSS into the JLabel
+            }
+        }
+        return component;
+    }
+
+    /**
+     * 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;
+    }
+}