diff --git a/cr-core/src/main/java/org/terasology/crashreporter/pages/BrowserLauncher.java b/cr-core/src/main/java/org/terasology/crashreporter/pages/BrowserLauncher.java new file mode 100644 index 0000000..fb56ebf --- /dev/null +++ b/cr-core/src/main/java/org/terasology/crashreporter/pages/BrowserLauncher.java @@ -0,0 +1,80 @@ +// Copyright 2026 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.crashreporter.pages; + +import org.terasology.crashreporter.I18N; + +import javax.swing.JOptionPane; +import java.awt.Desktop; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.function.Consumer; + +/** + * Opens a link in the user's default browser, used by every "open this URL" button in the dialog + * (Discord, GitHub issue, PasteBin result, ...). + * + *

Both {@code url} being {@code null}/empty (a {@link org.terasology.crashreporter.GlobalProperties} + * key that a downstream app never configured, or - for the GitHub button - was never filled in) and + * {@link java.awt.Desktop#browse} failing (no browser configured, unsupported platform, ...) used to + * throw straight out of a Swing {@code ActionListener} on the EDT: nothing catches that, so the + * button just did nothing with no trace anywhere the click ever happened. Every failure here is now + * caught, printed to stderr, and shown to the user - the same "never silently lose it" fix applied to + * PasteBin upload failures. + */ +final class BrowserLauncher { + + private BrowserLauncher() { + } + + static void open(String url) { + open(url, BrowserLauncher::browseWithDesktop); + } + + static void open(String url, Consumer browser) { + open(url, browser, BrowserLauncher::report); + } + + /** + * @param browser how to actually hand the parsed {@link URI} off to the platform, and + * @param onFailure what to do with a failure (missing/invalid url, or {@code browser} throwing) - + * package-private seam so tests can verify the null/blank-url guard and the exception + * handling without a real {@link java.awt.Desktop}/browser or popping up a real + * {@link JOptionPane}. + */ + static void open(String url, Consumer browser, Consumer onFailure) { + if (url == null || url.isEmpty()) { + onFailure.accept(new IllegalStateException("No link configured")); + return; + } + try { + browser.accept(new URI(url)); + } catch (URISyntaxException | RuntimeException e) { + onFailure.accept(e); + } + } + + private static void report(Exception e) { + e.printStackTrace(System.err); + JOptionPane.showMessageDialog(null, e.getLocalizedMessage(), I18N.getMessage("openLinkFailed"), + JOptionPane.ERROR_MESSAGE); + } + + private static void browseWithDesktop(URI uri) { + if (!Desktop.isDesktopSupported()) { + throw new IllegalStateException("Desktop integration is not supported on this platform"); + } + Desktop desktop = Desktop.getDesktop(); + if (!desktop.isSupported(Desktop.Action.BROWSE)) { + throw new IllegalStateException("Opening a browser is not supported on this platform"); + } + try { + desktop.browse(uri); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } +} diff --git a/cr-core/src/main/java/org/terasology/crashreporter/pages/FinalActionsPanel.java b/cr-core/src/main/java/org/terasology/crashreporter/pages/FinalActionsPanel.java index ebacd35..f9d452c 100644 --- a/cr-core/src/main/java/org/terasology/crashreporter/pages/FinalActionsPanel.java +++ b/cr-core/src/main/java/org/terasology/crashreporter/pages/FinalActionsPanel.java @@ -17,7 +17,6 @@ import javax.swing.SwingConstants; import javax.swing.border.EmptyBorder; import java.awt.BorderLayout; -import java.awt.Desktop; import java.awt.Dimension; import java.awt.Font; import java.awt.GridLayout; @@ -26,9 +25,6 @@ import java.awt.datatransfer.StringSelection; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; -import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; import java.net.URL; import java.util.function.Supplier; @@ -74,7 +70,7 @@ public FinalActionsPanel(GlobalProperties properties, Supplier uploadedFile @Override public void actionPerformed(ActionEvent e) { - openInBrowser(properties.get(KEY.JOIN_DISCORD_LINK)); + BrowserLauncher.open(properties.get(KEY.JOIN_DISCORD_LINK)); pageComplete = true; firePropertyChange("pageComplete", !pageComplete, pageComplete); } @@ -89,7 +85,7 @@ public void actionPerformed(ActionEvent e) { @Override public void actionPerformed(ActionEvent e) { - openInBrowser(properties.get(KEY.REPORT_ISSUE_LINK)); + BrowserLauncher.open(properties.get(KEY.REPORT_ISSUE_LINK)); pageComplete = true; firePropertyChange("pageComplete", !pageComplete, pageComplete); } @@ -104,7 +100,7 @@ public void actionPerformed(ActionEvent e) { @Override public void actionPerformed(ActionEvent e) { - openInBrowser(properties.get(KEY.SUPPORT_FORUM_LINK)); + BrowserLauncher.open(properties.get(KEY.SUPPORT_FORUM_LINK)); pageComplete = true; firePropertyChange("pageComplete", !pageComplete, pageComplete); } @@ -161,18 +157,4 @@ public void setVisible(boolean aFlag) { linkText.setText(text); linkText.setEnabled(log != null); } - - private static void openInBrowser(String url) { - if (Desktop.isDesktopSupported()) { - Desktop desktop = Desktop.getDesktop(); - - if (desktop.isSupported(Desktop.Action.BROWSE)) { - try { - desktop.browse(new URI(url)); - } catch (IOException | URISyntaxException e) { - e.printStackTrace(System.err); - } - } - } - } } diff --git a/cr-core/src/main/java/org/terasology/crashreporter/pages/UploadPanel.java b/cr-core/src/main/java/org/terasology/crashreporter/pages/UploadPanel.java index cc30939..5f85028 100644 --- a/cr-core/src/main/java/org/terasology/crashreporter/pages/UploadPanel.java +++ b/cr-core/src/main/java/org/terasology/crashreporter/pages/UploadPanel.java @@ -17,15 +17,12 @@ import javax.swing.border.EmptyBorder; import java.awt.BorderLayout; import java.awt.Cursor; -import java.awt.Desktop; import java.awt.Font; import java.awt.GridLayout; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; import java.awt.event.MouseAdapter; import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; import java.net.URL; import java.util.concurrent.Callable; import java.util.function.Supplier; @@ -145,7 +142,7 @@ private void updateStatus() { statusLabel.addMouseListener(new MouseAdapter() { @Override public void mouseClicked(java.awt.event.MouseEvent e) { - openInBrowser(uploadURL.toString()); + BrowserLauncher.open(uploadURL.toString()); } }); } else { @@ -181,18 +178,4 @@ public void run() { }); } - private static void openInBrowser(String url) { - if (Desktop.isDesktopSupported()) { - Desktop desktop = Desktop.getDesktop(); - - if (desktop.isSupported(Desktop.Action.BROWSE)) { - try { - desktop.browse(new URI(url)); - } catch (IOException | URISyntaxException e) { - e.printStackTrace(System.err); - } - } - } - } - } diff --git a/cr-core/src/main/resources/crashreporter_defaults.properties b/cr-core/src/main/resources/crashreporter_defaults.properties index a430772..af8a8c8 100644 --- a/cr-core/src/main/resources/crashreporter_defaults.properties +++ b/cr-core/src/main/resources/crashreporter_defaults.properties @@ -1,3 +1,6 @@ +REPORT_ISSUE_LINK=https://github.com/MovingBlocks/CrashReporter/issues/new +JOIN_DISCORD_LINK=https://discord.gg/terasology + RES_ARROW_PREV=icons/Arrow-Prev-icon.png RES_ARROW_NEXT=icons/Arrow-Next-icon.png RES_EXIT_ICON=icons/Actions-application-exit-icon.png diff --git a/cr-core/src/main/resources/i18n/MessagesBundle.properties b/cr-core/src/main/resources/i18n/MessagesBundle.properties index b6bef9f..74aa2f8 100644 --- a/cr-core/src/main/resources/i18n/MessagesBundle.properties +++ b/cr-core/src/main/resources/i18n/MessagesBundle.properties @@ -20,6 +20,7 @@ close=Close waitForUpload=Uploading file - please wait ... uploadComplete=Paste uploaded to uploadFailed=Upload failed +openLinkFailed=Could not open link uploadDialog=Upload to Pastebin gotoForum=Go to Support Forum next=Next diff --git a/cr-core/src/test/java/org/terasology/crashreporter/pages/BrowserLauncherTest.java b/cr-core/src/test/java/org/terasology/crashreporter/pages/BrowserLauncherTest.java new file mode 100644 index 0000000..4ac9766 --- /dev/null +++ b/cr-core/src/test/java/org/terasology/crashreporter/pages/BrowserLauncherTest.java @@ -0,0 +1,75 @@ +// Copyright 2026 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.crashreporter.pages; + +import org.junit.jupiter.api.Test; + +import java.net.URI; +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Regression test for the "Join Discord"/"File an issue on GitHub" buttons silently doing nothing - + * found while manually testing the reporter dialog. Both buttons call this class with a + * {@code GlobalProperties} key that cr-core's own defaults never set (only downstream apps like + * cr-terasology configure real Discord/issue-tracker links), so {@code url} was {@code null}; the old + * code passed that straight into {@code new URI(null)}, which throws an uncaught + * {@code NullPointerException} out of the button's {@code ActionListener} on the EDT - nothing catches + * that, so the click just did nothing, with no trace anywhere it happened. + */ +class BrowserLauncherTest { + + @Test + void aMissingUrlIsReportedNotThrown() { + List failures = new ArrayList<>(); + BrowserLauncher.open(null, uri -> unexpected(uri), failures::add); + + assertEquals(1, failures.size()); + } + + @Test + void aBlankUrlIsReportedNotThrown() { + List failures = new ArrayList<>(); + BrowserLauncher.open("", uri -> unexpected(uri), failures::add); + + assertEquals(1, failures.size()); + } + + @Test + void anInvalidUrlIsReportedNotThrown() { + List failures = new ArrayList<>(); + // an unescaped space makes this an invalid URI, not just an unreachable one + BrowserLauncher.open("not a valid uri", uri -> unexpected(uri), failures::add); + + assertEquals(1, failures.size()); + } + + @Test + void aBrowserFailureIsReportedNotThrown() { + List failures = new ArrayList<>(); + RuntimeException browserFailure = new IllegalStateException("no browser configured"); + BrowserLauncher.open("https://github.com/MovingBlocks/CrashReporter", uri -> { + throw browserFailure; + }, failures::add); + + assertEquals(1, failures.size()); + assertTrue(failures.get(0) == browserFailure); + } + + @Test + void aValidUrlIsHandedToTheBrowser() { + List opened = new ArrayList<>(); + BrowserLauncher.open("https://github.com/MovingBlocks/CrashReporter", opened::add, failure -> unexpected(failure)); + + assertEquals(1, opened.size()); + assertEquals("https://github.com/MovingBlocks/CrashReporter", opened.get(0).toString()); + } + + private static void unexpected(Object value) { + throw new AssertionError("Did not expect this to be reached: " + value); + } +}