Skip to content

Bug: external entity card HTTP(S) URL links open the OS browser instead of web tabs #1621

Closed

Description

@JoshuaRowePhantom

Summary

Clicking an HTTP(S) URL property on an external entity card launches the operating-system browser instead of opening a web tab in Phantom.Workspaces. This affects the single default URL and every named URL in an entity's urls map. Card links should use the existing in-app URL opener, keeping supported non-web handlers (e.g., mailto: / tel:) external rather than embedding them.

Root Cause

features/Phantom.Workspaces/ViewModels/ExternalEntityCardViewModel.cs:10-25 constructs each ExternalUrlViewModel with a command that directly shell-launches its captured URL and silently swallows failures:

this.OpenCommand = new RelayCommand(_ =>
{
    try
    {
        Process.Start(new ProcessStartInfo { FileName = url, UseShellExecute = true });
    }
    catch (Exception)
    {
        // Best-effort: if the URL cannot be opened, silently ignore.
    }
});

features/Phantom.Workspaces/Templates/WorkspaceDataTemplates.axaml:492-501 binds every external URL button to this command. ExternalEntityCardViewModel.Create enumerates all parsed URLs without an opener (ExternalEntityCardViewModel.cs:39-46); EntityCardViewModel.cs:93,775 calls it both initially and after snapshot changes. Thus the card never reaches IUrlOpener. The already-registered UrlOpener.OpenAsync(new OpenUrlRequest(url)) routes HTTP(S) to a focused embedded WebViewModel tab, reusing an existing same-URL tab in the selected pane (Services/UrlOpener.cs:70-111); other schemes currently go to its external launcher. App.axaml.cs:335-341 registers that service, and UsageTrackerViewModel.cs:117-140 demonstrates injecting an opener provider and calling it from a link command.

Affected Files

File Role
features/Phantom.Workspaces/ViewModels/ExternalEntityCardViewModel.cs Card URL commands currently bypass canonical routing; Create handles the entire urls map.
features/Phantom.Workspaces/ViewModels/EntityCardViewModel.cs Builds and rebuilds external cards; needs to pass an opener provider into both paths.
features/Phantom.Workspaces/ViewModels/EntityListNodeViewModel.cs and card construction call sites May need plumbing so cards can obtain the app's registered opener without directly launching URLs.
features/Phantom.Workspaces/Templates/WorkspaceDataTemplates.axaml Binds the clickable URL to each card URL's OpenCommand; retain or adjust binding as appropriate.
features/Phantom.Workspaces/Services/IUrlOpener.cs, Services/UrlOpener.cs Existing canonical routing, pane-aware same-URL focus/reuse and external scheme path; reuse rather than introduce a second browser mechanism.
features/Phantom.Workspaces.Tests/ExternalEntityCardViewModelTests.cs Existing naming/coverage for default and named URL properties; add command-routing tests.

Design / Fix

  • Supply the registered IUrlOpener (prefer a late-bound Func<IUrlOpener?>, as in UsageTrackerViewModel, because App.axaml.cs registers it after MainWindowViewModel construction) to external URL commands via the card creation/rebuild path. Do not create tabs or call Process.Start inside the card. For each URL property, call/await opener.OpenAsync(new OpenUrlRequest(url)) with the default Auto preference; ensure exceptions are observed/reported rather than silently swallowed. Illustrative shape:

    // URL command, with the same injected provider for every parsed entry:
    var opener = urlOpenerProvider?.Invoke();
    if (opener is not null)
        await opener.OpenAsync(new OpenUrlRequest(url));
  • HTTP and HTTPS (including default and all named properties) open/focus an embedded tab in the current workspace pane through the existing IUrlOpener implementation; repeated same-URL clicks focus the existing tab rather than spawning duplicates. mailto: and tel: remain external via the opener. Do not use Preference.External for web card URLs or change the separate explicit Open in External Browser affordance.

  • Validate the card's untrusted URL values before invoking any OS handler: ignore/disable empty or malformed values and reject or visibly report unsupported/unsafe schemes instead of shell-launching them. Note that today's UrlOpener routes all non-HTTP(S) schemes externally (including vscode:/file:), so it does not itself provide this safeguard; scope card-specific validation or coordinate a safe opener policy without breaking intentional existing integrations. If no opener is available, do not fall back to direct Process.Start; handle the missing service and failures consistently with existing app UX.

  • Preserve existing label presentation (single default key hidden; named/multiple keys shown), binding and snapshot-refresh behavior. This issue concerns clicking links on the external entity card, not the separate shortcut that opens the external entity itself.

Expected Tests

Test Name Class What It Verifies
ExternalEntityCardViewModel_DefaultHttpUrl_OpensEmbeddedWebTab ExternalEntityCardViewModelTests Clicking a single default HTTP URL reaches the canonical opener with Auto preference and opens an embedded tab, not the OS browser.
ExternalEntityCardViewModel_NamedHttpsUrls_OpenEmbeddedWebTabs ExternalEntityCardViewModelTests Every named URL command (not only default) routes through the injected opener.
ExternalEntityCardViewModel_SameUrlAlreadyOpen_FocusesExistingWebTab ExternalEntityCardViewModelTests Repeated activation uses the opener's current-pane same-URL focus/reuse instead of a new tab.
ExternalEntityCardViewModel_MailtoAndTelUrls_UseExternalLauncher ExternalEntityCardViewModelTests Supported non-web links are delegated to the canonical external route, not embedded.
ExternalEntityCardViewModel_InvalidOrUnsafeUrl_DoesNotLaunch ExternalEntityCardViewModelTests Invalid or disallowed card properties do not invoke an OS handler and are handled visibly/consistently.
EntityCardViewModel_ExternalSnapshotUpdated_UpdatedUrlsUseUrlOpener ExternalEntityCardViewModelTests Rebuilt cards after snapshot changes retain opener routing and the expected key labels.

Activity

  1. JoshuaRowePhantom commented on Oct 8, 2026

    @JoshuaRowePhantom
    OwnerAuthor

    Verification result

    Status: ❌ 3 issue(s) found
    Commits checked: 91c4a688 Fix #1621: route external card URLs through in-app opener

    Outstanding gaps

    • Production launch failures remain silent. UrlOpener.CreateDefault ignores a false result from TopLevel.Launcher.LaunchUriAsync, and its Process.Start fallback catches and suppresses exceptions. Consequently the new card clears ErrorMessage despite an unsuccessful mailto:/tel: launch; the issue requires failures to be surfaced. The injected-throwing-opener test does not exercise the production launcher.
    • Actual application wiring is untested. The command tests pass an opener directly to ExternalEntityCardViewModel.Create or EntityCardViewModel, but the production EntityWorkspaceTabViewModel.EntityCardNode path relies on SetShortcutContext and the late-populated ApplicationServices.UrlOpener fallback. Add a command test for that end-to-end path, including registration after card construction and snapshot refresh.
    • Visual behavior is untested. No test realizes the external-card AXAML template, clicks the bound URL button, or checks the visible error text / disabled invalid URL button and default/named key presentation. The issue requests visual/command verification, and the new HasError binding has no rendering test.

    Tests

    worktrees\17\scripts\run-tests.ps1 -Mode fast -TestNames ExternalEntityCardViewModelTests: Executed: 13; Passed: 13; Failed: 0 (scripts\test-results.log). Initial run took approximately 14 minutes because the solution run built the unrelated Install.Tests wrapper publish prerequisite; it eventually completed successfully, with no hang dump. No filesystem/Git repository-layer files changed; slow-suite label not applicable.

  2. JoshuaRowePhantom commented on Oct 8, 2026

    @JoshuaRowePhantom
    OwnerAuthor

    Verification result

    Status: ❌ 1 issue(s) found
    Commits checked: 05806a3e Verify external card wiring and surface launcher failures; 91c4a688 Fix #1621: route external card URLs through in-app opener

    Previously reported gaps

    • Production launcher failure now falls back on LaunchUriAsync == false, propagates shell-launch exceptions, and the card displays the failure. Covered by ExternalCard_ProductionLauncherFails_ReportsFailure and CreateDefault_ShellLauncherFails_PropagatesFailure.
    • EntityWorkspaceTab_ExternalCard_LateRegisteredOpenerAndSnapshotRefresh exercises the real card-node/shortcut-context/application-services path, including registration after construction and default/named URL refresh.
    • ExternalCardTemplate_BoundButtonsShowErrorsAndPreserveLabels realizes the AXAML template, clicks a URL button, verifies the visible failure, disabled unsupported link, invalid-link error, and default/named labels.

    Outstanding gaps

    • The changed production launcher has an untested successful LaunchUriAsync branch (UrlOpener.CreateDefault: if (launched) return). Existing tests cover launcher false and shell failure, but none verifies that launcher true returns without invoking the shell fallback. The shared branch-coverage criterion requires a test for this path.

    Tests

    worktrees\17\scripts\run-tests.ps1 -Mode fast -TestNames ExternalEntityCardViewModelTests,UrlOpenerTests: Executed: 31; Passed: 31; Failed: 0 (scripts\test-results.log). No filesystem/Git repository-layer changes; slow-suite label not applicable.

  3. JoshuaRowePhantom commented on Oct 8, 2026

    @JoshuaRowePhantom
    OwnerAuthor

    Verification result

    Status: ✅ Verified
    Commits checked: 859e40be Test successful external URI launch skips shell fallback; 05806a3e Verify external card wiring and surface launcher failures; 91c4a688 Fix #1621: route external card URLs through in-app opener (against features).

    Previously reported gaps

    • CreateDefault_LauncherReturnsTrue_DoesNotUseShellFallback now exercises the successful launcher return in the actual UrlOpener.CreateDefault external-launch flow using injected launcher and shell delegates: the launcher receives the mailto URI, while shell fallback and workspace-tab methods are never called. Production LaunchUriAsync and injected launch delegate share this same launched/return branch.
    • Production launcher-failure propagation, late application-service registration and snapshot refresh, and realized AXAML button/error/key presentation remain covered by the previously added tests. Card URL validation restricts OS handlers; HTTP(S) uses the canonical Auto opener with same-pane tab reuse. No new disabled tests, unresolved TODOs, timing-based synchronization or synchronous UI task blocking found.

    Outstanding gaps

    None — all criteria satisfied.

    Tests

    worktrees\17\scripts\run-tests.ps1 -Mode fast -TestNames ExternalEntityCardViewModelTests,UrlOpenerTests: scripts\test-results.log reports Executed: 32; Passed: 32; Failed: 0 (Skipped: 0). No filesystem or Git repository-layer changes; slow tests not needed.

  4. JoshuaRowePhantom commented on Oct 9, 2026

    @JoshuaRowePhantom
    OwnerAuthor

    Checked into features via #1626 (merge 1b86a98). Hosted fast CI 37966216282 passed 8,019/8,019; full CI 37966209695 passed on Windows and Linux. #1617 and #1618 retain their separate acceptance gates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingdiagnosedRoot cause identifiedverified-locallyImplementation has been verified locally

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions