From ae1cf7f54fe1ffc1ebf4569a58fc67297c83b33a Mon Sep 17 00:00:00 2001 From: mmtr <1233880+mmtr@users.noreply.github.com> Date: Fri, 21 Aug 2026 14:03:44 +0200 Subject: [PATCH 1/4] Dock: keep off-site admin menus out, and put the wp-admin originals back A host can extend the admin menu with links to its own control panel, and can repoint Core entries at its own versions of those screens. Nothing on another host loads in a window, so those tiles could only ever escape to a browser tab. The builder now drops off-site entries, honours the hide-if-js class, and restores a hidden wp-admin row when the off-site row that displaced it is dropped. A menu with no page of its own and no child left to open is dropped too. Icons are sampled across admin_menu so art moved into a stylesheet mid-hook still reaches the dock. Co-Authored-By: Claude Opus 5 --- assets/css/openstation-layout.css | 8 + docs/hooks-reference.md | 29 ++ docs/javascript-reference.md | 8 +- docs/plugin-compat-layer.md | 17 +- includes/core/payload.php | 434 +++++++++++++++--- src/desktop-layout.ts | 6 + src/dock-constellation/index.ts | 14 + src/dock.ts | 12 +- src/types.ts | 7 +- src/window/dom-external-tabs.test.ts | 73 +++ src/window/dom.ts | 16 +- .../tests/openStationExternalMenuItems.php | 394 ++++++++++++++++ tests/vitest/dock-constellation.test.ts | 39 ++ 13 files changed, 992 insertions(+), 65 deletions(-) create mode 100644 src/window/dom-external-tabs.test.ts create mode 100644 tests/phpunit/tests/openStationExternalMenuItems.php diff --git a/assets/css/openstation-layout.css b/assets/css/openstation-layout.css index 868bd988f..efc334507 100644 --- a/assets/css/openstation-layout.css +++ b/assets/css/openstation-layout.css @@ -493,6 +493,14 @@ body.os-constellation-open .os-dock__tooltip { opacity: 0.6; } +.os-constellation__row-external { + flex: 0 0 auto; + font-size: 14px; + width: 14px; + height: 14px; + opacity: 0.55; +} + /* ---- Head -------------------------------------------------------- */ .os-constellation__head { diff --git a/docs/hooks-reference.md b/docs/hooks-reference.md index a6c5ddfba..6c16736bb 100644 --- a/docs/hooks-reference.md +++ b/docs/hooks-reference.md @@ -1215,6 +1215,10 @@ Items built from the admin menu also carry `selfLabel`, `multi`, `placement`, `i **`submenu` excludes the menu's own page.** WordPress auto-prepends a self-link to every parent menu (`All Posts` → `edit.php`, the same URL as the parent), and the builder strips it so `count( $submenu )` reliably means "how many distinct child pages" — the in-window tab strip would otherwise grow a duplicate first tab, and the right-click popover keys its suppression off an empty list. The stripped entry's label survives on **`selfLabel`** (`''` when the menu had none), so a surface that *lists* a menu's pages can put the main page back where wp-admin has it. The constellation flyout does exactly that, pointing the row at the item's `url`. +**Off-site menu entries never reach the dock.** Nothing on another host can load in a window, so a menu whose URL points off-site is dropped rather than turned into a tile that can only escape to a browser tab. The one exception is a child of a menu a regular plugin registered (`pluginFile` is non-null) whose own URL stays on-site: those keep their row and carry `'external' => true`, which the constellation marks as leaving the site and the in-window tab strip skips. The classifier is [`openstation_menu_item_is_external`](#openstation_menu_item_is_external--stable). + +**Rows a host hid stay hidden, unless dropping them would lose the page.** A `$menu` / `$submenu` row carrying the `hide-if-js` class is out of the classic sidebar and out of the dock too. WordPress.com is why the rule has an exception: rather than repoint a Core entry at wordpress.com, Jetpack marks the wp-admin original `hide-if-js` and appends a Calypso duplicate beside it. When the duplicate is dropped as off-site, the original takes its place in the list — so Appearance → Themes, Plugins → Add Plugin and Users → All Users open the wp-admin screens Core registered. + **Example — add a virtual dock item:** ```php @@ -1323,6 +1327,31 @@ The live menu-refresh path (chromeless `plugins.php` iframe postMessage, plus th --- +### `openstation_menu_item_is_external` — Stable + +Whether a resolved admin-menu URL counts as off-site. Off-site entries are dropped from the dock payload (see [`openstation_dock_items`](#openstation_dock_items--stable) for the exception plugin menus get), because nothing on another host can load in a window. + +```php +apply_filters( 'openstation_menu_item_is_external', bool $external, string $url ); +``` + +By default a URL is off-site when its host matches neither `admin_url()`'s nor `home_url()`'s. Both count, so a site running its admin on a separate domain from its front end isn't misread. + +**Example — keep a trusted sibling domain in the dock:** + +```php +add_filter( 'openstation_menu_item_is_external', function ( $external, $url ) { + if ( str_contains( $url, 'admin.internal.example.com' ) ) { + return false; + } + return $external; +}, 10, 2 ); +``` + +An entry you allow back in still has to survive the browser: a host that sends `X-Frame-Options` or a `frame-ancestors` policy refuses the iframe whatever this filter says. + +--- + ### `openstation_arrange_menu_items` — Stable The list of plugin-contributed items appended to the admin bar's **Arrange** submenu — the dropdown that sits next to the "Switch to…" toggle when OpenStation is active. Built-ins (Cascade, Overview, Snap to grid, Tile all windows) are always present; this filter adds to them. Only invoked when the user is viewing the desktop shell. diff --git a/docs/javascript-reference.md b/docs/javascript-reference.md index da5f686fe..e797ae525 100644 --- a/docs/javascript-reference.md +++ b/docs/javascript-reference.md @@ -857,7 +857,7 @@ manager.closeDesktop( id: string ): void; width?: number; height?: number; initialState?: 'normal' | 'minimized' | 'maximized' | 'fullscreen'; - submenu?: { title: string; url: string }[]; + submenu?: { title: string; url: string; external?: boolean }[]; } ``` @@ -911,7 +911,7 @@ Ownership is about z-order and focus. For a *visual* relationship between peer w To react to a blocked focus attempt, subscribe to [`os-window-child-blocked`](#os-window-child-blocked) or the `os.window.child-blocked` action. -**`config.submenu`** — when present, the shell renders the array as an in-window tab strip below the title bar so the user can navigate child pages without leaving the window. Pass `item.submenu` whenever you open a window from a dock context — `openItem` and `openSubmenuPick` (in custom rail renderers) propagate it for you. Skip it for native windows that don't have admin sub-pages. The shell strips WordPress's auto-prepended self-link entry server-side, so `submenu.length > 0` reliably means "has real children" (no defensive filtering needed in your code). The shell prepends a synthetic "back to parent" tab (label = `config.title`, URL = `config.url`) as the first tab so the user can return to the parent listing without closing the window. If a caller-supplied submenu entry already points at `config.url` the synthetic tab is suppressed to avoid two tabs claiming the same URL. +**`config.submenu`** — when present, the shell renders the array as an in-window tab strip below the title bar so the user can navigate child pages without leaving the window. Entries flagged `external` are skipped: a tab loads its URL into this window's iframe, which an off-site origin refuses. Pass `item.submenu` whenever you open a window from a dock context — `openItem` and `openSubmenuPick` (in custom rail renderers) propagate it for you. Skip it for native windows that don't have admin sub-pages. The shell strips WordPress's auto-prepended self-link entry server-side, so `submenu.length > 0` reliably means "has real children" (no defensive filtering needed in your code). The shell prepends a synthetic "back to parent" tab (label = `config.title`, URL = `config.url`) as the first tab so the user can return to the parent listing without closing the window. If a caller-supplied submenu entry already points at `config.url` the synthetic tab is suppressed to avoid two tabs claiming the same URL. Every iframe window gets the strip element, whether or not it has a submenu, because external sub-tabs can be added to it later. Its navigation semantics follow its contents: `role="tablist"` plus an `aria-label` of `" sub-pages"` while it holds tabs, `role="presentation"` while it is empty — so a window with no sub-pages never advertises an empty tab list to assistive tech. @@ -4798,7 +4798,7 @@ interface DockItem { icon: string; // dashicon class | `data:` URI | `http(s):` URL url: string; // admin URL the tile opens badge: number; // numeric badge; 0 = no badge - submenu: { title: string; url: string }[]; + submenu: { title: string; url: string; external?: boolean }[]; multi: boolean; // hover-peek + Ghost Card eligibility isCore: boolean; // true for WP-shipped menus, false for plugin-contributed pluginFile: string | null; // owning plugin file (e.g. `woocommerce/woocommerce.php`) @@ -4827,6 +4827,8 @@ interface DockItem { A custom rail renderer that decides whether to show a submenu indicator (a chevron, a hover treatment) can read `item.submenu.length > 0` without defensive `submenu.length > 1` or self-URL filtering. The framework owns the contract. +**`submenu[].external`** — the row leaves the site. Off-site admin-menu entries are dropped server-side; the survivors are children of a plugin's own menu (a docs or account link), and they carry this flag. Nothing off-site can load in an iframe, so a surface that routes a URL into a window must skip them — the in-window tab strip does, and the constellation flyout marks them with an outbound glyph and hands them to the browser instead. `tryOpenExternalUrl()` is the shared escape; a renderer calling `openSubmenuPick` gets it for free. + **Lifecycle pairing — `replaceItems` ↔ `appendSystemItem`** — these are independent update paths. `replaceItems( items )` swaps the menu-derived tiles wholesale (the live menu refresh fires it on every plugin activation / deactivation). `appendSystemItem` / `removeSystemItem` track the JS-owned cohort (OpenStation Preferences, plugin native-window launchers). A custom rail renderer's controller MUST persist its system-tile DOM across `replaceItems` calls — the shell does NOT re-emit `appendSystemItem` for previously-added tiles after a menu refresh. Practical pattern: track system tiles in a closure-scoped `Map`, re-paint them in `replaceItems()` after rebuilding the menu cohort. diff --git a/docs/plugin-compat-layer.md b/docs/plugin-compat-layer.md index c791e6e54..c8d70969a 100644 --- a/docs/plugin-compat-layer.md +++ b/docs/plugin-compat-layer.md @@ -116,10 +116,25 @@ Dock URLs flow into the shell config as JSON, then end up assigned to `iframe.sr Some plugins register a top-level menu with a stub callback whose actual landing page is the first submenu (`add_menu_page( …, 'woocommerce', null, … )` then `add_submenu_page( 'woocommerce', …, 'wc-admin', … )`). Classic admin's `wp-admin/menu-header.php` rewrites the parent's clickable link to the first submenu's URL. Hitting `?page=woocommerce` directly invokes the stub and 500s. -**Fix**: `openstation_build_dock_items()` mirrors this — if a parent menu has any visible submenu, the parent's effective URL is the first capability-passing submenu's URL. +**Fix**: `openstation_build_dock_items()` mirrors this — if a parent menu has any visible submenu, the parent's effective URL is the first capability-passing submenu's URL. A menu that registered a self-link keeps its own URL instead, wherever in the list that link sits: it has a working page of its own, and the fallthrough is only for menus that don't. **Plugins this addresses**: WooCommerce, historically Yoast SEO, several membership / LMS plugins. +### Admin menus that point at another host (WordPress.com) + +A site can be hosted somewhere that extends the admin menu with links to its own control panel. WordPress.com is the case that surfaced this: My Home, Theme Showcase, Hosting and friends are `wordpress.com` URLs, and Jetpack also repoints Core entries — Appearance → Themes, Plugins → Add Plugin, Users → All Users — at their Calypso counterparts. None of them can load in a window; the remote origin refuses the frame. Routing them to a browser tab instead, which is what the shell used to do, takes the user out of the desktop on a click that looked like every other dock click. + +**Fix**, all in `openstation_build_dock_items()`: + +1. Any menu whose resolved URL is off-site is dropped. `openstation_menu_item_is_external()` is the classifier, and it is filterable. +2. A child of a menu a regular plugin registered (`pluginFile` non-null) is the exception — a docs or account link under a plugin's own menu is a normal thing to ship. It keeps its row, flagged `external`, which the constellation marks with an outbound glyph and the in-window tab strip skips. +3. Rows carrying `hide-if-js` are dropped, which is also what removes the duplicate submenus WordPress.com produced: Jetpack keeps the wp-admin original and marks it hidden rather than replacing it, so the dock was rendering both copies. +4. Except when that hidden row is the original of an off-site row we just dropped — then it takes the dropped row's place in the list, and the menu opens the wp-admin screen Core registered. This is what puts Themes, Add Plugin and All Users back. + +**Icons**: `Base_Admin_Menu::override_svg_icons()` moves every SVG-data-URI menu icon into an inline stylesheet and sets `$menu[ $i ][6]` to `'none'`, which left Jetpack, MailPoet and every other plugin shipping vector art with a generic gear in the dock. `openstation_snapshot_menu_icons()` samples `$menu` at several points across `admin_menu` and records, write-once, the first real icon each slug wore; the builder falls back to it when the live value has been blanked. Sampling rather than parking one priority below the known rewriter is deliberate — registrations and rewrites both happen at arbitrary priorities, and the live value still wins whenever there is one, so a menu that genuinely changes its icon is unaffected. + +Nothing here is WordPress.com-specific: the rules read the menu arrays, not the host. + ### Empty submenu titles Plugins (notably WooCommerce's `wc-addons` Extensions row) register `menu_title => null` to keep a page reachable while hiding the row from classic admin's left menu. Our dock would otherwise render an empty, label-less tab that visually duplicates a sibling entry. diff --git a/includes/core/payload.php b/includes/core/payload.php index 87941396d..922bffde5 100644 --- a/includes/core/payload.php +++ b/includes/core/payload.php @@ -55,6 +55,14 @@ function openstation_build_dock_items() { continue; } + // Skip menus something took out of the classic sidebar. A dock + // that shows what wp-admin hides isn't a faithful mirror of the + // menu, and on WordPress.com it double-renders every entry + // Jetpack replaced with a Calypso link. + if ( openstation_menu_item_is_hidden( $item ) ) { + continue; + } + $title = openstation_menu_item_title( $item[0] ); // Extract badge count from the title HTML. @@ -84,7 +92,19 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) // Determine the icon. Menu entries can set `$item[6]` to anything // — a dashicon class, a remote URL, a data:URI, 'none', or 'div' // — so normalize before we serialize it for the shell JS. - $icon = openstation_sanitize_dock_icon( $item[6] ?? '' ); + // + // A blanked value falls back to whatever the row carried before + // anything on `admin_menu` rewrote it, which is how plugin + // artwork survives Jetpack's SVG-to-stylesheet move on + // WordPress.com — see `openstation_snapshot_menu_icons()`. + $raw_icon = (string) ( $item[6] ?? '' ); + if ( '' === $raw_icon || 'none' === $raw_icon || 'div' === $raw_icon ) { + $snapshot = openstation_menu_icon_snapshot(); + if ( isset( $snapshot[ $item[2] ] ) ) { + $raw_icon = $snapshot[ $item[2] ]; + } + } + $icon = openstation_sanitize_dock_icon( $raw_icon ); // Build the full URL for the menu item. // @@ -94,8 +114,17 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) // The effective `$url` we ship to the shell can be rewritten // further down to the first visible submenu's URL — see the // note after the loop. - $parent_url = openstation_menu_item_url( $item[2] ); - $url = $parent_url; + $parent_url = openstation_menu_item_url( $item[2] ); + $parent_external = openstation_menu_item_is_external( $parent_url ); + + // A menu owned by a regular plugin is allowed to keep off-site + // children — a docs or support link under a plugin's own menu is + // a normal thing to ship, and the flyout marks it as leaving the + // site. Everything else drops them: a Core menu whose child was + // repointed off-site (WordPress.com does this to Appearance → + // Themes) gets its wp-admin original back instead, below. + $plugin_file = openstation_resolve_menu_plugin_file( $item[2] ); + $allow_external_subs = null !== $plugin_file && ! $parent_external; // Build submenu items. // @@ -114,9 +143,16 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) // Detection by URL (post-`openstation_menu_item_url()` normalize) // rather than slug equality covers plugins that register a child // at a different slug pointing at the parent's URL. - $sub_items = array(); - $first_visible_sub_url = null; - $self_label = ''; + // + // Two passes, because the second decision depends on the first: + // a `hide-if-js` row is normally noise, but when it is the + // wp-admin original of an off-site row we just dropped, it is + // the route back to the page Core intended. The original takes + // the replacement's place in the list, so the menu reads the way + // it would have if nothing had swapped the row out. + $rows = array(); + $restore_slots = array(); + $dropped_off_site = 0; if ( ! empty( $submenu[ $item[2] ] ) ) { foreach ( $submenu[ $item[2] ] as $sub_item ) { if ( ! empty( $sub_item[1] ) && ! current_user_can( $sub_item[1] ) ) { @@ -128,59 +164,159 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) // when `<body class=\"no-customize-support\">`". The // Customizer is supported inside chromeless iframes, so // these entries belong in the dock. - $sub_url = openstation_menu_item_url( $sub_item[2] ); - // Capture the first capability-passing submenu URL so - // we can use it as the parent's effective URL below - // (mirrors `wp-admin/menu-header.php`). Captured BEFORE - // the self-link strip so plugins whose first submenu IS - // the auto-prepended self-link land on the parent URL - // (a no-op rewrite — preserves existing behavior). - if ( null === $first_visible_sub_url ) { - $first_visible_sub_url = $sub_url; - } - // Self-link strip — `$sub_url === $parent_url` covers - // WP's auto-prepended entry AND any plugin-registered - // alias that happens to land on the parent URL. - if ( $sub_url === $parent_url ) { - // Keep its LABEL, though. The stripped entry is a - // real row in wp-admin's own menu ("All Posts", - // "All Pages"), and the constellation flyout lists - // it as the first thing the menu opens — a list of - // a menu's pages that omits its main page reads as - // a bug. - // - // Carried separately rather than left in `submenu` - // because `submenu` has two other consumers that - // need it to mean "distinct child links only": the - // in-window tab strip, which would grow a duplicate - // first tab, and the right-click popover, which is - // suppressed on `length === 0`. - // - // First one only — a plugin can register several - // aliases onto the parent URL, and the canonical - // self-link is the one WordPress prepends. - if ( '' === $self_label ) { - $self_label = openstation_menu_item_title( $sub_item[0] ); + $sub_url = openstation_menu_item_url( $sub_item[2] ); + $sub_external = openstation_menu_item_is_external( $sub_url ); + + if ( $sub_external && ! $allow_external_subs ) { + ++$dropped_off_site; + // Leave a slot behind, in case the wp-admin row this + // entry displaced is still in the list. + $dropped_title = openstation_menu_item_title( $sub_item[0] ); + if ( '' !== $dropped_title && ! isset( $restore_slots[ $dropped_title ] ) ) { + $rows[] = array( 'restore' => $dropped_title ); + $restore_slots[ $dropped_title ] = count( $rows ) - 1; } continue; } - // Skip entries with no resolvable title. Plugins (e.g. - // WooCommerce's `wc-addons` Extensions row) register - // `menu_title => null` to hide a row from classic admin's - // left menu while keeping the page reachable. Without - // this guard the dock renders an empty, label-less tab - // that visually duplicates a sibling entry. - $sub_title = openstation_menu_item_title( $sub_item[0] ); - if ( '' === $sub_title ) { - continue; - } - $sub_items[] = array( - 'title' => $sub_title, - 'url' => $sub_url, + + $rows[] = array( + 'raw_title' => $sub_item[0], + 'url' => $sub_url, + 'external' => $sub_external, + 'hidden' => openstation_menu_item_is_hidden( $sub_item ), ); } } + // Second pass. A hidden row moves into the slot its replacement + // left; one whose replacement was the top-level slug itself + // stays where it is (there is no slot — the menu row is not part + // of this list). Every other hidden row, and every slot nothing + // claimed, drops out. + $restored = array(); + $keep = array_fill( 0, count( $rows ), true ); + foreach ( $rows as $i => $row ) { + if ( isset( $row['restore'] ) || ! $row['hidden'] ) { + continue; + } + $keep[ $i ] = false; + $row_title = openstation_menu_item_title( $row['raw_title'] ); + if ( '' === $row_title || isset( $restored[ $row_title ] ) ) { + continue; + } + if ( isset( $restore_slots[ $row_title ] ) ) { + $rows[ $restore_slots[ $row_title ] ] = $row; + $restored[ $row_title ] = true; + } elseif ( $parent_external && $row_title === $title ) { + $keep[ $i ] = true; + $restored[ $row_title ] = true; + } + } + $kept_rows = array(); + foreach ( $rows as $i => $row ) { + if ( isset( $row['restore'] ) || ! $keep[ $i ] ) { + continue; + } + $kept_rows[] = $row; + } + $rows = $kept_rows; + + // When the top-level slug itself points off-site, the menu's + // identity is now whichever child survived — adopt it before the + // self-link strip runs, so a restored original collapses into + // `selfLabel` instead of becoming a child that duplicates its + // own parent. + if ( $parent_external ) { + foreach ( $rows as $row ) { + if ( ! $row['external'] ) { + $parent_url = $row['url']; + break; + } + } + } + + // A menu that only ever pointed at its children, and whose + // children we just took away. Checked only for menus the + // off-site rule actually touched, so a menu registering its page + // hook in some way we don't recognise is left exactly as it was. + $parent_is_container = $dropped_off_site > 0 + && ! $parent_external + && ! openstation_menu_slug_has_page( $item[2] ); + + $url = $parent_url; + $sub_items = array(); + $first_visible_sub_url = null; + $has_self_link = false; + $self_label = ''; + foreach ( $rows as $row ) { + $sub_url = $row['url']; + if ( $parent_is_container && $sub_url === $parent_url ) { + // A row pointing back at a menu with no page is a dead + // end, not a way back — it can't name the menu and it + // can't stand in for it. + continue; + } + // Capture the first capability-passing submenu URL so + // we can use it as the parent's effective URL below + // (mirrors `wp-admin/menu-header.php`). Captured BEFORE + // the self-link strip so plugins whose first submenu IS + // the auto-prepended self-link land on the parent URL + // (a no-op rewrite — preserves existing behavior). Never + // an off-site child, which would take the whole tile with + // it when the final external check runs. + if ( null === $first_visible_sub_url && ! $row['external'] ) { + $first_visible_sub_url = $sub_url; + } + // Self-link strip — `$sub_url === $parent_url` covers + // WP's auto-prepended entry AND any plugin-registered + // alias that happens to land on the parent URL. + if ( $sub_url === $parent_url ) { + $has_self_link = true; + // Keep its LABEL, though. The stripped entry is a + // real row in wp-admin's own menu ("All Posts", + // "All Pages"), and the constellation flyout lists + // it as the first thing the menu opens — a list of + // a menu's pages that omits its main page reads as + // a bug. + // + // Carried separately rather than left in `submenu` + // because `submenu` has two other consumers that + // need it to mean "distinct child links only": the + // in-window tab strip, which would grow a duplicate + // first tab, and the right-click popover, which is + // suppressed on `length === 0`. + // + // First one only — a plugin can register several + // aliases onto the parent URL, and the canonical + // self-link is the one WordPress prepends. + if ( '' === $self_label ) { + $self_label = openstation_menu_item_title( $row['raw_title'] ); + } + continue; + } + // Skip entries with no resolvable title. Plugins (e.g. + // WooCommerce's `wc-addons` Extensions row) register + // `menu_title => null` to hide a row from classic admin's + // left menu while keeping the page reachable. Without + // this guard the dock renders an empty, label-less tab + // that visually duplicates a sibling entry. + $sub_title = openstation_menu_item_title( $row['raw_title'] ); + if ( '' === $sub_title ) { + continue; + } + $sub_entry = array( + 'title' => $sub_title, + 'url' => $sub_url, + ); + if ( $row['external'] ) { + // Consumers that route a URL into a window skip these; + // the ones that can hand a link to the browser mark + // them as leaving the site. + $sub_entry['external'] = true; + } + $sub_items[] = $sub_entry; + } + // Mirror `wp-admin/menu-header.php`: when a parent menu has any // visible submenu, classic admin rewrites the parent's // clickable URL to the first submenu's URL. Plugins like @@ -190,10 +326,33 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) // (`?page=wc-admin` for WC). Without this rewrite the dock // icon points users at a broken URL that classic admin would // never have linked to. - if ( null !== $first_visible_sub_url ) { + // + // A menu that registered a self-link has a working page of its + // own and keeps it, wherever in the list that link sits. Only + // the WooCommerce shape — no self-link at all — needs a child to + // stand in. Position matters here because a restored wp-admin + // row inherits the slot its off-site replacement held, which on + // WordPress.com puts `plugin-install.php` first under Plugins. + if ( null !== $first_visible_sub_url && ! $has_self_link ) { $url = $first_visible_sub_url; } + // Nothing on this menu resolves to a page we can open. Hosts + // that link their own control panel from the admin menu + // (WordPress.com's My Home, Theme Showcase, Hosting) land here, + // and so does a Core menu whose slug was repointed off-site with + // no wp-admin child left to fall back to. + if ( openstation_menu_item_is_external( $url ) ) { + continue; + } + + // A container menu with nothing left to stand in for it. Its + // URL resolves to core's "Cannot load <slug>." page, which is a + // worse tile than no tile. + if ( $parent_is_container && $url === $parent_url ) { + continue; + } + $dock_item = array( 'id' => sanitize_key( $item[5] ?? $item[2] ), 'title' => $title, @@ -209,7 +368,7 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) 'multi' => openstation_dock_item_is_multi( $item[2] ), 'placement' => openstation_dock_placement( $item[2] ), 'isCore' => openstation_is_core_menu_slug( $item[2] ), - 'pluginFile' => openstation_resolve_menu_plugin_file( $item[2] ), + 'pluginFile' => $plugin_file, 'pluginName' => null, ); if ( $dock_item['pluginFile'] ) { @@ -235,6 +394,173 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) return apply_filters( 'openstation_dock_items', $items ); } +/** + * Whether a resolved menu URL points at a host other than this site's. + * + * OpenStation opens admin pages inside iframes, and an off-site URL + * cannot load in one — the remote origin's `X-Frame-Options` / + * `frame-ancestors` header refuses it. Hosts that extend the admin + * menu with links to their own control panel (WordPress.com registers + * My Home, Theme Showcase, Hosting and friends as `wordpress.com` + * URLs) would therefore fill the dock with tiles that can only ever + * escape to a browser tab, which breaks the shell's navigation model. + * Those entries are dropped from the payload instead. + * + * Both `admin_url()` and `home_url()` hosts count as ours: a site can + * run its admin on a different domain than its front end. + * + * @param string $url Absolute URL, as returned by `openstation_menu_item_url()`. + * @return bool True when the URL is off-site. + */ +function openstation_menu_item_is_external( $url ) { + $host = wp_parse_url( (string) $url, PHP_URL_HOST ); + $external = false; + + if ( $host ) { + $ours = array(); + foreach ( array( admin_url(), home_url() ) as $known ) { + $known_host = wp_parse_url( $known, PHP_URL_HOST ); + if ( $known_host ) { + $ours[] = strtolower( $known_host ); + } + } + $external = ! in_array( strtolower( $host ), $ours, true ); + } + + /** + * Filters whether an admin-menu URL counts as off-site. + * + * @param bool $external Whether the URL points off-site. + * @param string $url The resolved menu URL. + */ + return (bool) apply_filters( 'openstation_menu_item_is_external', $external, $url ); +} + +/** + * Whether a `$menu` / `$submenu` row carries the `hide-if-js` class. + * + * Core never sets it on a menu row, so it reads as "some other code + * took this entry out of the sidebar". Jetpack's admin-menu + * customisation on WordPress.com uses it heavily: rather than replace + * a Core entry with its wordpress.com counterpart, it marks the + * original `hide-if-js` and appends a duplicate pointing at Calypso. + * Honouring the class is what keeps those pairs from rendering twice + * in the dock. + * + * @param array $item A `$menu` or `$submenu` row. + * @return bool True when the row is hidden from the classic sidebar. + */ +function openstation_menu_item_is_hidden( $item ) { + return ! empty( $item[4] ) && false !== strpos( (string) $item[4], 'hide-if-js' ); +} + +/** + * Whether a top-level menu slug has a page of its own behind it. + * + * `add_menu_page()` accepts a `null` callback, which registers a menu + * that is nothing but a container for its children — WordPress links + * such a parent to its first submenu and `admin.php` refuses the slug + * directly with "Cannot load <slug>." WordPress.com's Upgrades menu is + * one: `paid-upgrades.php` has no callback and no self-link, and every + * child is a wordpress.com URL. Drop the children and the tile is left + * pointing at core's error page. + * + * Two ways a slug earns a page: it names a real file under `wp-admin/`, + * or something is listening on its page hook — the same `has_action()` + * test `get_plugin_page_hook()` makes before `admin.php` gives up. + * Anything we can't answer counts as a page, so an unusual registration + * costs a menu nothing. + * + * @param string $slug The menu slug from `$menu[$i][2]`. + * @return bool False only when the slug is provably a container. + */ +function openstation_menu_slug_has_page( $slug ) { + if ( openstation_is_admin_file_slug( $slug ) ) { + return true; + } + + if ( ! function_exists( 'get_plugin_page_hookname' ) ) { + return true; + } + + $hookname = get_plugin_page_hookname( $slug, '' ); + if ( empty( $hookname ) ) { + return true; + } + + return has_action( $hookname ); +} + +/** + * Lazy accessor for the pre-rewrite menu icon snapshot: `slug → icon`. + * + * Populated by {@see openstation_snapshot_menu_icons()}. + * + * @return array<string,string> + */ +function &openstation_menu_icon_snapshot() { + static $map = null; + if ( null === $map ) { + $map = array(); + } + return $map; +} + +/** + * Record the first real icon each menu row is seen wearing. + * + * A menu row's icon is not final when it is registered. Anything on + * `admin_menu` can rewrite `$menu[ $i ][6]`, and the rewrite that hurts + * is to `'none'` — the row keeps its picture in the sidebar, painted + * from a stylesheet instead, and the menu array stops carrying it. The + * dock reads the array, so those menus arrived wearing a generic gear. + * Jetpack's `override_svg_icons()` does this to every SVG-data-URI icon + * on WordPress.com, which is where it was found, but nothing about the + * move is specific to that host. + * + * Rather than sit at one priority chosen to undercut one known rewriter, + * sample repeatedly and **never overwrite**: the map keeps the earliest + * real icon each slug had, whenever it appeared and whoever blanked it + * afterwards. Write-once is safe because the map is only ever consulted + * as a fallback — a menu that genuinely changes its icon still ships the + * live value. + * + * A slug that had no real icon at any sample point is simply absent, and + * the caller lands on the generic fallback it would have had anyway. + */ +function openstation_snapshot_menu_icons() { + global $menu; + + if ( ! is_array( $menu ) ) { + return; + } + + $map = &openstation_menu_icon_snapshot(); + + foreach ( $menu as $item ) { + if ( empty( $item[2] ) || empty( $item[6] ) ) { + continue; + } + $slug = (string) $item[2]; + if ( isset( $map[ $slug ] ) ) { + continue; + } + $icon = (string) $item[6]; + if ( 'none' === $icon || 'div' === $icon ) { + continue; + } + $map[ $slug ] = $icon; + } +} +// Spread across the hook rather than parked just below any one +// rewriter: registrations and rewrites both happen at arbitrary +// priorities, and only a sample taken before a given rewrite can see +// what it overwrote. +foreach ( array( 11, 100, 1000, 99998, PHP_INT_MAX ) as $openstation_icon_snapshot_priority ) { + add_action( 'admin_menu', 'openstation_snapshot_menu_icons', $openstation_icon_snapshot_priority ); +} +unset( $openstation_icon_snapshot_priority ); + /** * Sanitizes a dock icon value for safe injection into the shell JS. * diff --git a/src/desktop-layout.ts b/src/desktop-layout.ts index 0bb2e35ac..bcd3b1945 100644 --- a/src/desktop-layout.ts +++ b/src/desktop-layout.ts @@ -41,6 +41,7 @@ import { import type { WindowManager } from './window-manager'; import { deriveWindowId } from './utils'; import { resolveNativeUrlRemap } from './native-url-remap'; +import { tryOpenExternalUrl } from './external-url'; import type { DesktopLayoutId, DockPlacementId, @@ -627,6 +628,11 @@ export function createLayoutDispatcher( } ); }, openSubmenuPick: ( item, sub ) => { + // A plugin's off-site child can't be iframed — hand it to + // the browser, the same way the constellation row does. + if ( tryOpenExternalUrl( sub.url ) ) { + return; + } deps.windowManager.open( { id: deriveWindowId( sub.url, deps.adminUrl ), baseId: deriveWindowId( item.url, deps.adminUrl ), diff --git a/src/dock-constellation/index.ts b/src/dock-constellation/index.ts index 575518a94..62da177eb 100644 --- a/src/dock-constellation/index.ts +++ b/src/dock-constellation/index.ts @@ -1095,6 +1095,20 @@ function buildSubmenuRow( label.textContent = sub.title; row.appendChild( label ); + // An off-site row can't become a window — clicking it hands the URL + // to the browser. Say so before the click, not after it. + if ( sub.external ) { + const mark = document.createElement( 'span' ); + mark.className = 'dashicons dashicons-external os-constellation__row-external'; + mark.setAttribute( 'aria-hidden', 'true' ); + row.appendChild( mark ); + row.setAttribute( + 'aria-label', + // translators: %s is the submenu entry's label (e.g. "Documentation") + sprintf( __( '%s (opens in a new tab)' ), sub.title ), + ); + } + row.addEventListener( 'click', () => { dismiss(); runRow( deps, item, sub ); diff --git a/src/dock.ts b/src/dock.ts index 6738a1dad..68c2a59e6 100644 --- a/src/dock.ts +++ b/src/dock.ts @@ -222,6 +222,16 @@ export interface SubmenuItem { * unset. */ windowId?: string; + /** + * This row leaves the site. + * + * Set server-side on a plugin menu's off-site children — a docs or + * account link under the plugin's own menu. Nothing off-site can + * load in a window, so surfaces that route a URL into one skip + * these rows, and the ones that can hand a link to the browser mark + * them as leaving. + */ + external?: boolean; } export interface DockItem { @@ -260,7 +270,7 @@ export interface DockItem { /** Number badge (update count, comment count, etc.). 0 = no badge. */ badge: number; /** Submenu items. */ - submenu: { title: string; url: string }[]; + submenu: { title: string; url: string; external?: boolean }[]; /** Whether this admin page supports multiple open windows. */ multi?: boolean; /** diff --git a/src/types.ts b/src/types.ts index 7e9e81f9b..f72489b67 100644 --- a/src/types.ts +++ b/src/types.ts @@ -131,8 +131,11 @@ export interface WindowConfig { * Submenu items that render as a tab strip below the title bar. * Each tab navigates the iframe within the same window — no new window opens. * Pass an empty array (or omit) to hide the strip. + * + * Rows flagged `external` are carried through but never become + * tabs: the iframe can't load an off-site URL. */ - submenu?: { title: string; url: string }[]; + submenu?: { title: string; url: string; external?: boolean }[]; /** * Optional initial state. When present, the window is constructed * into this state directly — used by session restore so a minimized @@ -1597,7 +1600,7 @@ export interface DockItemConfig { /** Badge count (updates, comments, etc.). */ badge: number; /** Submenu items. */ - submenu: { title: string; url: string }[]; + submenu: { title: string; url: string; external?: boolean }[]; /** * WordPress's own label for this menu's landing page ("Themes", * "All Posts"), stripped out of `submenu` as the self-link. Names diff --git a/src/window/dom-external-tabs.test.ts b/src/window/dom-external-tabs.test.ts new file mode 100644 index 000000000..364f8cd7d --- /dev/null +++ b/src/window/dom-external-tabs.test.ts @@ -0,0 +1,73 @@ +/** + * A window's tab strip loads each tab's URL into its own iframe, so a + * row pointing at another host has no tab to be — the remote origin + * refuses the frame. `createWindowElement` drops those rows. + */ +import { afterEach, beforeEach, describe, expect, test } from 'vitest'; +import { createWindowElement } from './dom'; +import type { WindowConfig } from '../types'; +import { + clearHooksStub, + installHooksStub, +} from '../../tests/vitest/helpers/hooks-stub'; + +const ADMIN = window.location.origin + '/wp-admin/'; + +function build( submenu: WindowConfig[ 'submenu' ] ): HTMLElement { + return createWindowElement( { + id: 'my-plugin', + title: 'My Plugin', + url: ADMIN + 'admin.php?page=my-plugin', + icon: 'dashicons-admin-generic', + x: 0, + y: 0, + width: 800, + height: 600, + minWidth: 320, + minHeight: 240, + submenu, + } as WindowConfig ); +} + +/** Labels of the strip's submenu tabs, in order. */ +function tabLabels( el: HTMLElement ): string[] { + return Array.from( + el.querySelectorAll< HTMLElement >( '.os-window__tab[data-kind="submenu"]' ), + ).map( ( tab ) => tab.textContent ?? '' ); +} + +describe( 'tab strip and off-site submenu rows', () => { + beforeEach( () => { + installHooksStub(); + } ); + + afterEach( () => { + clearHooksStub(); + } ); + + test( 'an external row never becomes a tab', () => { + const el = build( [ + { title: 'Settings', url: ADMIN + 'admin.php?page=my-plugin-settings' }, + { title: 'Docs', url: 'https://example.org/docs', external: true }, + ] ); + + expect( tabLabels( el ) ).toEqual( [ 'My Plugin', 'Settings' ] ); + } ); + + test( 'a submenu of nothing but external rows renders no strip', () => { + const el = build( [ + { title: 'Docs', url: 'https://example.org/docs', external: true }, + ] ); + + expect( tabLabels( el ) ).toEqual( [] ); + } ); + + test( 'internal rows are untouched', () => { + const el = build( [ + { title: 'Settings', url: ADMIN + 'admin.php?page=my-plugin-settings' }, + { title: 'Tools', url: ADMIN + 'admin.php?page=my-plugin-tools' }, + ] ); + + expect( tabLabels( el ) ).toEqual( [ 'My Plugin', 'Settings', 'Tools' ] ); + } ); +} ); diff --git a/src/window/dom.ts b/src/window/dom.ts index f9bd6a2ec..fe8c87ff1 100644 --- a/src/window/dom.ts +++ b/src/window/dom.ts @@ -847,7 +847,15 @@ export function createWindowElement( config: WindowConfig ): HTMLElement { tabs.dataset.tablistLabel = sprintf( __( '%s sub-pages' ), config.title ); } - if ( config.submenu && config.submenu.length > 0 && config.url ) { + // Off-site rows never become tabs: a tab loads its URL into + // this window's iframe, and the remote origin refuses the + // frame. They stay in the constellation flyout, which can hand + // a link to the browser. + const tabSubmenu = ( config.submenu ?? [] ).filter( + ( s ) => ! s.external, + ); + + if ( tabSubmenu.length > 0 && config.url ) { const initialKey = urlMatchKey( config.url ); // Synthetic "back to parent" tab — `helpers.php` strips WP's @@ -873,7 +881,7 @@ export function createWindowElement( config: WindowConfig ): HTMLElement { // "Add Theme" entry but `parentUrl = themes.php` doesn't). const synthUrl = config.parentUrl ?? config.url; const synthKey = urlMatchKey( synthUrl ); - const parentAlreadyInSubmenu = config.submenu.some( + const parentAlreadyInSubmenu = tabSubmenu.some( ( s ) => urlMatchKey( s.url ) === synthKey, ); // Labelled the way WordPress labels it — "Themes" under @@ -882,10 +890,10 @@ export function createWindowElement( config: WindowConfig ): HTMLElement { // and the fallback for menus with no self-link to take a // name from. const seedSubmenu: { title: string; url: string }[] = parentAlreadyInSubmenu - ? [ ...config.submenu ] + ? [ ...tabSubmenu ] : [ { title: config.selfLabel || config.title, url: synthUrl }, - ...config.submenu, + ...tabSubmenu, ]; for ( const sub of seedSubmenu ) { diff --git a/tests/phpunit/tests/openStationExternalMenuItems.php b/tests/phpunit/tests/openStationExternalMenuItems.php new file mode 100644 index 000000000..ac26cb192 --- /dev/null +++ b/tests/phpunit/tests/openStationExternalMenuItems.php @@ -0,0 +1,394 @@ +<?php +/** + * Tests for how the dock payload treats admin-menu entries that point + * off-site, and for the wp-admin originals a host leaves behind when it + * swaps one in. + * + * @package WordPress + * @subpackage UnitTests + * + * @group openstation + * + * @covers ::openstation_build_dock_items + * @covers ::openstation_menu_item_is_external + * @covers ::openstation_menu_item_is_hidden + */ +class Tests_OpenStation_ExternalMenuItems extends WP_UnitTestCase { + + protected static $admin_id; + + protected $original_menu; + protected $original_submenu; + + public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { + self::$admin_id = $factory->user->create( array( 'role' => 'administrator' ) ); + } + + public function set_up() { + parent::set_up(); + global $menu, $submenu; + $this->original_menu = $menu; + $this->original_submenu = $submenu; + $menu = array(); + $submenu = array(); + wp_set_current_user( self::$admin_id ); + } + + public function tear_down() { + global $menu, $submenu; + $menu = $this->original_menu; + $submenu = $this->original_submenu; + + $map = &openstation_menu_attribution_map(); + $map = array(); + + $icons = &openstation_menu_icon_snapshot(); + $icons = array(); + + remove_all_filters( 'openstation_menu_item_is_external' ); + parent::tear_down(); + } + + private function make_menu_row( $title, $cap, $slug, $page_title = '', $classes = '', $hookname = '', $icon = 'dashicons-admin-post' ) { + return array( + $title, + $cap, + $slug, + $page_title, + $classes, + $hookname ? $hookname : 'menu-' . sanitize_key( str_replace( '.', '-', $slug ) ), + $icon, + ); + } + + /** Make `$slug` resolve to a regular plugin, the way the admin_menu tracker would. */ + private function attribute_to_plugin( $slug, $plugin_file = 'my-plugin/my-plugin.php' ) { + $map = &openstation_menu_attribution_map(); + $map[ $slug ] = $plugin_file; + } + + public function test_site_urls_are_not_external() { + $this->assertFalse( openstation_menu_item_is_external( admin_url( 'edit.php' ) ) ); + $this->assertFalse( openstation_menu_item_is_external( home_url( '/' ) ) ); + } + + public function test_off_site_url_is_external() { + $this->assertTrue( + openstation_menu_item_is_external( 'https://wordpress.com/home/example.com' ) + ); + } + + public function test_external_filter_can_reclassify_a_url() { + add_filter( 'openstation_menu_item_is_external', '__return_false' ); + $this->assertFalse( + openstation_menu_item_is_external( 'https://wordpress.com/home/example.com' ) + ); + } + + public function test_top_level_menu_pointing_off_site_is_dropped() { + global $menu; + $menu = array( + $this->make_menu_row( 'Posts', 'edit_posts', 'edit.php' ), + $this->make_menu_row( 'My Home', 'read', 'https://wordpress.com/home/example.com' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertCount( 1, $items ); + $this->assertSame( 'Posts', $items[0]['title'] ); + } + + public function test_off_site_submenu_of_a_core_menu_is_dropped() { + global $menu, $submenu; + $menu = array( $this->make_menu_row( 'Tools', 'edit_posts', 'tools.php' ) ); + $submenu['tools.php'] = array( + array( 'Available Tools', 'edit_posts', 'tools.php' ), + array( 'Hosting', 'manage_options', 'https://wordpress.com/hosting/example.com' ), + array( 'Import', 'import', 'import.php' ), + ); + + $items = openstation_build_dock_items(); + + $titles = wp_list_pluck( $items[0]['submenu'], 'title' ); + $this->assertSame( array( 'Import' ), $titles ); + } + + public function test_off_site_submenu_of_a_plugin_menu_is_kept_and_flagged() { + global $menu, $submenu; + $this->attribute_to_plugin( 'my-plugin' ); + + $menu = array( $this->make_menu_row( 'My Plugin', 'manage_options', 'my-plugin' ) ); + $submenu['my-plugin'] = array( + array( 'Settings', 'manage_options', 'my-plugin' ), + array( 'Help', 'manage_options', 'https://example.org/docs' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertCount( 1, $items[0]['submenu'] ); + $this->assertSame( 'Help', $items[0]['submenu'][0]['title'] ); + $this->assertTrue( $items[0]['submenu'][0]['external'] ); + } + + public function test_internal_submenu_is_not_flagged_external() { + global $menu, $submenu; + $menu = array( $this->make_menu_row( 'Posts', 'edit_posts', 'edit.php' ) ); + $submenu['edit.php'] = array( + array( 'All Posts', 'edit_posts', 'edit.php' ), + array( 'Add New', 'edit_posts', 'post-new.php' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertArrayNotHasKey( 'external', $items[0]['submenu'][0] ); + } + + public function test_off_site_submenu_never_becomes_the_parent_url() { + global $menu, $submenu; + $this->attribute_to_plugin( 'my-plugin' ); + + $menu = array( $this->make_menu_row( 'My Plugin', 'manage_options', 'my-plugin' ) ); + $submenu['my-plugin'] = array( + array( 'Upgrade', 'manage_options', 'https://example.org/pricing' ), + array( 'Settings', 'manage_options', 'my-plugin-settings' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertSame( + admin_url( 'admin.php?page=my-plugin-settings' ), + $items[0]['url'] + ); + } + + public function test_hidden_submenu_entries_are_dropped() { + global $menu, $submenu; + $menu = array( $this->make_menu_row( 'Tools', 'edit_posts', 'tools.php' ) ); + $submenu['tools.php'] = array( + array( 'Available Tools', 'edit_posts', 'tools.php' ), + array( 'Import', 'import', 'import.php', '', 'hide-if-js' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertSame( array(), $items[0]['submenu'] ); + } + + public function test_hidden_original_is_restored_when_its_off_site_replacement_is_dropped() { + global $menu, $submenu; + // The shape Jetpack leaves behind on WordPress.com: the wp-admin + // row marked `hide-if-js`, a Calypso duplicate carrying the same + // label added in front of it. + $menu = array( $this->make_menu_row( 'Appearance', 'switch_themes', 'themes.php' ) ); + $submenu['themes.php'] = array( + array( 'Themes', 'switch_themes', 'https://wordpress.com/themes/example.com' ), + array( 'Editor', 'edit_theme_options', 'site-editor.php' ), + array( 'Themes', 'switch_themes', 'themes.php', '', 'hide-if-js' ), + ); + + $items = openstation_build_dock_items(); + + // The restored row is the parent's own page, so it collapses + // into `selfLabel` rather than becoming a child. + $this->assertSame( 'Themes', $items[0]['selfLabel'] ); + $this->assertSame( admin_url( 'themes.php' ), $items[0]['url'] ); + // `Add Theme` is OpenStation's own injected Appearance tab. + $titles = wp_list_pluck( $items[0]['submenu'], 'title' ); + $this->assertContains( 'Editor', $titles ); + $this->assertNotContains( 'Themes', $titles ); + } + + public function test_hidden_original_restores_a_child_page() { + global $menu, $submenu; + $menu = array( $this->make_menu_row( 'Plugins', 'activate_plugins', 'plugins.php' ) ); + $submenu['plugins.php'] = array( + array( 'Installed Plugins', 'activate_plugins', 'plugins.php' ), + array( 'Add New Plugin', 'install_plugins', 'https://wordpress.com/plugins/example.com' ), + array( 'Add New Plugin', 'install_plugins', 'plugin-install.php', '', 'hide-if-js' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertSame( + array( 'Add New Plugin' ), + wp_list_pluck( $items[0]['submenu'], 'title' ) + ); + $this->assertSame( + admin_url( 'plugin-install.php' ), + $items[0]['submenu'][0]['url'] + ); + } + + public function test_top_level_menu_pointing_off_site_falls_back_to_its_restored_child() { + global $menu, $submenu; + // `Base_Admin_Menu::update_menu()` rewrites the top-level slug + // itself when the menu has no visible children left, hiding the + // self-link on the way past. + $menu = array( $this->make_menu_row( 'Plugins', 'activate_plugins', 'https://wordpress.com/plugins/example.com' ) ); + $submenu['https://wordpress.com/plugins/example.com'] = array( + array( 'Plugins', 'activate_plugins', 'plugins.php', '', 'hide-if-js' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertCount( 1, $items ); + $this->assertSame( admin_url( 'plugins.php' ), $items[0]['url'] ); + $this->assertSame( 'Plugins', $items[0]['selfLabel'] ); + } + + public function test_restored_child_does_not_steal_the_parent_url() { + global $menu, $submenu; + // The Plugins shape on WordPress.com: Jetpack moves Add New to + // the top, points it at Calypso, and leaves the wp-admin row + // behind at the end of the list. Restoring it must not make + // "Plugins" open the installer. + $menu = array( $this->make_menu_row( 'Plugins', 'activate_plugins', 'plugins.php' ) ); + $submenu['plugins.php'] = array( + array( 'Add Plugin', 'install_plugins', 'https://wordpress.com/plugins/example.com' ), + array( 'Installed Plugins', 'activate_plugins', 'plugins.php' ), + array( 'Add Plugin', 'install_plugins', 'plugin-install.php', '', 'hide-if-js' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertSame( admin_url( 'plugins.php' ), $items[0]['url'] ); + $this->assertSame( 'Installed Plugins', $items[0]['selfLabel'] ); + $this->assertSame( + array( 'Add Plugin' ), + wp_list_pluck( $items[0]['submenu'], 'title' ) + ); + } + + public function test_container_menu_whose_children_were_all_off_site_is_dropped() { + global $menu, $submenu; + // The WordPress.com Upgrades shape: `add_menu_page()` with a + // null callback, its self-link removed, and every child a + // wordpress.com URL. Keeping the tile would point it at core's + // "Cannot load paid-upgrades.php." page. + $menu = array( + $this->make_menu_row( 'Posts', 'edit_posts', 'edit.php' ), + $this->make_menu_row( 'Upgrades', 'manage_options', 'paid-upgrades.php' ), + ); + $submenu['paid-upgrades.php'] = array( + array( 'Plans', 'manage_options', 'https://wordpress.com/plans/example.com' ), + array( 'Domains', 'manage_options', 'https://wordpress.com/domains/manage/example.com' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertSame( array( 'Posts' ), wp_list_pluck( $items, 'title' ) ); + } + + public function test_container_menu_keeps_an_on_site_child_to_stand_in() { + global $menu, $submenu; + $menu = array( $this->make_menu_row( 'Upgrades', 'manage_options', 'paid-upgrades.php' ) ); + $submenu['paid-upgrades.php'] = array( + array( 'Plans', 'manage_options', 'https://wordpress.com/plans/example.com' ), + array( 'Receipts', 'manage_options', 'my-receipts' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertCount( 1, $items ); + $this->assertSame( admin_url( 'admin.php?page=my-receipts' ), $items[0]['url'] ); + } + + public function test_container_menu_with_a_registered_page_is_kept() { + global $menu, $submenu; + // Same shape, but something is listening on the page hook — the + // menu renders, so it keeps its tile even with no children left. + $menu = array( $this->make_menu_row( 'My Plugin', 'manage_options', 'my-plugin' ) ); + $submenu['my-plugin'] = array( + array( 'Account', 'manage_options', 'https://example.org/account' ), + ); + $hookname = get_plugin_page_hookname( 'my-plugin', '' ); + add_action( $hookname, '__return_null' ); + + try { + $items = openstation_build_dock_items(); + } finally { + remove_action( $hookname, '__return_null' ); + } + + $this->assertCount( 1, $items ); + $this->assertSame( admin_url( 'admin.php?page=my-plugin' ), $items[0]['url'] ); + } + + public function test_container_check_leaves_menus_without_off_site_children_alone() { + global $menu; + // No off-site row was dropped here, so the container check never + // runs and a callback-less menu keeps behaving as it always did. + $menu = array( $this->make_menu_row( 'Empty', 'manage_options', 'empty-menu' ) ); + + $items = openstation_build_dock_items(); + + $this->assertCount( 1, $items ); + $this->assertSame( admin_url( 'admin.php?page=empty-menu' ), $items[0]['url'] ); + } + + public function test_hidden_top_level_menu_is_dropped() { + global $menu; + $menu = array( + $this->make_menu_row( 'Posts', 'edit_posts', 'edit.php' ), + $this->make_menu_row( 'Gone', 'read', 'gone', '', 'menu-top hide-if-js' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertSame( array( 'Posts' ), wp_list_pluck( $items, 'title' ) ); + } + + public function test_blanked_icon_falls_back_to_the_snapshot() { + global $menu; + $svg = 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4='; + $menu = array( $this->make_menu_row( 'Jetpack', 'manage_options', 'jetpack', '', '', '', $svg ) ); + + // Snapshot while the icon is still an icon, then let something + // on `admin_menu` blank it the way Jetpack's SVG override does. + openstation_snapshot_menu_icons(); + $menu[0][6] = 'none'; + + $items = openstation_build_dock_items(); + + $this->assertSame( $svg, $items[0]['icon'] ); + } + + public function test_snapshot_keeps_the_first_icon_a_slug_wore() { + global $menu; + $svg = 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4='; + $menu = array( $this->make_menu_row( 'Thing', 'manage_options', 'thing', '', '', '', $svg ) ); + + // Sampled once while the icon is real, again after a rewrite — + // the second pass must not record the blank over the first. + openstation_snapshot_menu_icons(); + $menu[0][6] = 'none'; + openstation_snapshot_menu_icons(); + + $items = openstation_build_dock_items(); + + $this->assertSame( $svg, $items[0]['icon'] ); + } + + public function test_live_icon_wins_over_the_snapshot() { + global $menu; + $menu = array( $this->make_menu_row( 'Thing', 'manage_options', 'thing', '', '', '', 'dashicons-cart' ) ); + + openstation_snapshot_menu_icons(); + // A menu that genuinely swaps its icon still ships the new one; + // the snapshot is a fallback, not an override. + $menu[0][6] = 'dashicons-chart-bar'; + + $items = openstation_build_dock_items(); + + $this->assertSame( 'dashicons-chart-bar', $items[0]['icon'] ); + } + + public function test_blanked_icon_without_a_snapshot_stays_generic() { + global $menu; + $menu = array( $this->make_menu_row( 'Jetpack', 'manage_options', 'jetpack', '', '', '', 'none' ) ); + + $items = openstation_build_dock_items(); + + $this->assertSame( 'dashicons-admin-generic', $items[0]['icon'] ); + } +} diff --git a/tests/vitest/dock-constellation.test.ts b/tests/vitest/dock-constellation.test.ts index fa5af00eb..57e72e541 100644 --- a/tests/vitest/dock-constellation.test.ts +++ b/tests/vitest/dock-constellation.test.ts @@ -321,6 +321,45 @@ describe( 'dock constellation', () => { ).toBe( true ); } ); + test( 'an off-site row is marked, and leaves for the browser', () => { + const openSpy = vi.fn(); + vi.stubGlobal( 'open', openSpy ); + + const tile = setupShell( 'openstation' ); + mountWith( [ + { + ...appearance, + submenu: [ + { title: 'Editor', url: '/wp-admin/site-editor.php' }, + { + title: 'Docs', + url: 'https://example.org/docs', + external: true, + }, + ], + }, + ] ); + hover( tile ); + + const docs = rows( '.os-constellation__row--sub' )[ 1 ]; + expect( + docs.querySelector( '.os-constellation__row-external' ), + ).not.toBeNull(); + expect( docs.getAttribute( 'aria-label' ) ).toBe( + 'Docs (opens in a new tab)', + ); + + docs.click(); + // A window would have loaded it into an iframe the remote + // origin refuses; the browser gets it instead. + expect( opened ).toHaveLength( 0 ); + expect( openSpy ).toHaveBeenCalledWith( + 'https://example.org/docs', + '_blank', + 'noopener,noreferrer', + ); + } ); + test( 'head opens the menu; a submenu row opens its child page', () => { const tile = setupShell( 'openstation' ); mount(); From 12e8b515f8b878db69b97682fd3d283cae34d03f Mon Sep 17 00:00:00 2001 From: mmtr <1233880+mmtr@users.noreply.github.com> Date: Fri, 21 Aug 2026 14:49:04 +0200 Subject: [PATCH 2/4] Dock: carry menu identity onto a rescued tile, and rename the row flag Adopting a surviving child's URL moved only the URL. Everything keyed off the menu's slug still read the off-site one, so a rescued Plugins tile reported isCore false, sorted into the plugin cohort at the end of the dock, and picked up whoever registered the replacement as its owning plugin. The openstation_dock_item filter saw the off-site slug too, which would have made openstation_inject_appearance_tabs() skip a rescued Appearance menu. Identity now travels with the URL. Renames the submenu row flag from external to offSite. The window's tab strip already uses data-kind="external" for plugin-opened sub-iframes, which are on-site, and the two meanings sat a few lines apart. The constellation's "opens in a new tab" note moves from an aria-label on the row to a visually hidden span, so it extends the accessible name instead of replacing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- assets/css/openstation-layout.css | 20 +++++++- docs/hooks-reference.md | 4 +- docs/javascript-reference.md | 10 ++-- docs/plugin-compat-layer.md | 4 +- includes/core/payload.php | 32 ++++++++++--- src/dock-constellation/index.ts | 17 ++++--- src/dock.ts | 10 ++-- src/types.ts | 6 +-- src/window/dom-external-tabs.test.ts | 4 +- src/window/dom.ts | 2 +- src/window/iframe-bridge.ts | 4 +- .../tests/openStationExternalMenuItems.php | 48 +++++++++++++++++++ tests/vitest/dock-constellation.test.ts | 11 +++-- 13 files changed, 134 insertions(+), 38 deletions(-) diff --git a/assets/css/openstation-layout.css b/assets/css/openstation-layout.css index efc334507..3aa2c6ce7 100644 --- a/assets/css/openstation-layout.css +++ b/assets/css/openstation-layout.css @@ -493,7 +493,7 @@ body.os-constellation-open .os-dock__tooltip { opacity: 0.6; } -.os-constellation__row-external { +.os-constellation__row-offsite { flex: 0 0 auto; font-size: 14px; width: 14px; @@ -501,6 +501,24 @@ body.os-constellation-open .os-dock__tooltip { opacity: 0.55; } +/* + * Reaches screen readers, never the eye. It rides beside the row's own + * label so the accessible name reads "Docs (opens in a new tab)"; an + * `aria-label` on the row would have replaced that name rather than + * extended it, taking the label and any plugin-appended meta with it. + */ +.os-constellation__row-note { + position: absolute; + width: 1px; + height: 1px; + margin: -1px; + padding: 0; + border: 0; + overflow: hidden; + white-space: nowrap; + clip-path: inset( 50% ); +} + /* ---- Head -------------------------------------------------------- */ .os-constellation__head { diff --git a/docs/hooks-reference.md b/docs/hooks-reference.md index 6c16736bb..ecbf66ac8 100644 --- a/docs/hooks-reference.md +++ b/docs/hooks-reference.md @@ -1215,7 +1215,9 @@ Items built from the admin menu also carry `selfLabel`, `multi`, `placement`, `i **`submenu` excludes the menu's own page.** WordPress auto-prepends a self-link to every parent menu (`All Posts` → `edit.php`, the same URL as the parent), and the builder strips it so `count( $submenu )` reliably means "how many distinct child pages" — the in-window tab strip would otherwise grow a duplicate first tab, and the right-click popover keys its suppression off an empty list. The stripped entry's label survives on **`selfLabel`** (`''` when the menu had none), so a surface that *lists* a menu's pages can put the main page back where wp-admin has it. The constellation flyout does exactly that, pointing the row at the item's `url`. -**Off-site menu entries never reach the dock.** Nothing on another host can load in a window, so a menu whose URL points off-site is dropped rather than turned into a tile that can only escape to a browser tab. The one exception is a child of a menu a regular plugin registered (`pluginFile` is non-null) whose own URL stays on-site: those keep their row and carry `'external' => true`, which the constellation marks as leaving the site and the in-window tab strip skips. The classifier is [`openstation_menu_item_is_external`](#openstation_menu_item_is_external--stable). +**Off-site menu entries never reach the dock.** Nothing on another host can load in a window, so a menu whose URL points off-site is dropped rather than turned into a tile that can only escape to a browser tab. The one exception is a child of a menu a regular plugin registered (`pluginFile` is non-null) whose own URL stays on-site: those keep their row and carry `'offSite' => true`, which the constellation marks as leaving the site and the in-window tab strip skips. The classifier is [`openstation_menu_item_is_external`](#openstation_menu_item_is_external--stable). + +**A rescued menu takes on the identity of the slug it adopted.** When a menu's own slug points off-site and a surviving child stands in for it, `multi`, `placement`, `isCore`, `pluginFile` and the `$menu_slug` passed to `openstation_dock_item` are all derived from that child's slug, not from the off-site one. Otherwise a rescued Plugins tile reports as a plugin menu owned by whoever registered the replacement. **Rows a host hid stay hidden, unless dropping them would lose the page.** A `$menu` / `$submenu` row carrying the `hide-if-js` class is out of the classic sidebar and out of the dock too. WordPress.com is why the rule has an exception: rather than repoint a Core entry at wordpress.com, Jetpack marks the wp-admin original `hide-if-js` and appends a Calypso duplicate beside it. When the duplicate is dropped as off-site, the original takes its place in the list — so Appearance → Themes, Plugins → Add Plugin and Users → All Users open the wp-admin screens Core registered. diff --git a/docs/javascript-reference.md b/docs/javascript-reference.md index e797ae525..2665bc3c9 100644 --- a/docs/javascript-reference.md +++ b/docs/javascript-reference.md @@ -857,7 +857,7 @@ manager.closeDesktop( id: string ): void; width?: number; height?: number; initialState?: 'normal' | 'minimized' | 'maximized' | 'fullscreen'; - submenu?: { title: string; url: string; external?: boolean }[]; + submenu?: { title: string; url: string; offSite?: boolean }[]; } ``` @@ -911,7 +911,7 @@ Ownership is about z-order and focus. For a *visual* relationship between peer w To react to a blocked focus attempt, subscribe to [`os-window-child-blocked`](#os-window-child-blocked) or the `os.window.child-blocked` action. -**`config.submenu`** — when present, the shell renders the array as an in-window tab strip below the title bar so the user can navigate child pages without leaving the window. Entries flagged `external` are skipped: a tab loads its URL into this window's iframe, which an off-site origin refuses. Pass `item.submenu` whenever you open a window from a dock context — `openItem` and `openSubmenuPick` (in custom rail renderers) propagate it for you. Skip it for native windows that don't have admin sub-pages. The shell strips WordPress's auto-prepended self-link entry server-side, so `submenu.length > 0` reliably means "has real children" (no defensive filtering needed in your code). The shell prepends a synthetic "back to parent" tab (label = `config.title`, URL = `config.url`) as the first tab so the user can return to the parent listing without closing the window. If a caller-supplied submenu entry already points at `config.url` the synthetic tab is suppressed to avoid two tabs claiming the same URL. +**`config.submenu`** — when present, the shell renders the array as an in-window tab strip below the title bar so the user can navigate child pages without leaving the window. Entries flagged `offSite` are skipped: a tab loads its URL into this window's iframe, which an off-site origin refuses. Pass `item.submenu` whenever you open a window from a dock context — `openItem` and `openSubmenuPick` (in custom rail renderers) propagate it for you. Skip it for native windows that don't have admin sub-pages. The shell strips WordPress's auto-prepended self-link entry server-side, so `submenu.length > 0` reliably means "has real children" (no defensive filtering needed in your code). The shell prepends a synthetic "back to parent" tab (label = `config.title`, URL = `config.url`) as the first tab so the user can return to the parent listing without closing the window. If a caller-supplied submenu entry already points at `config.url` the synthetic tab is suppressed to avoid two tabs claiming the same URL. Every iframe window gets the strip element, whether or not it has a submenu, because external sub-tabs can be added to it later. Its navigation semantics follow its contents: `role="tablist"` plus an `aria-label` of `"<title> sub-pages"` while it holds tabs, `role="presentation"` while it is empty — so a window with no sub-pages never advertises an empty tab list to assistive tech. @@ -4798,7 +4798,7 @@ interface DockItem { icon: string; // dashicon class | `data:` URI | `http(s):` URL url: string; // admin URL the tile opens badge: number; // numeric badge; 0 = no badge - submenu: { title: string; url: string; external?: boolean }[]; + submenu: { title: string; url: string; offSite?: boolean }[]; multi: boolean; // hover-peek + Ghost Card eligibility isCore: boolean; // true for WP-shipped menus, false for plugin-contributed pluginFile: string | null; // owning plugin file (e.g. `woocommerce/woocommerce.php`) @@ -4827,7 +4827,9 @@ interface DockItem { A custom rail renderer that decides whether to show a submenu indicator (a chevron, a hover treatment) can read `item.submenu.length > 0` without defensive `submenu.length > 1` or self-URL filtering. The framework owns the contract. -**`submenu[].external`** — the row leaves the site. Off-site admin-menu entries are dropped server-side; the survivors are children of a plugin's own menu (a docs or account link), and they carry this flag. Nothing off-site can load in an iframe, so a surface that routes a URL into a window must skip them — the in-window tab strip does, and the constellation flyout marks them with an outbound glyph and hands them to the browser instead. `tryOpenExternalUrl()` is the shared escape; a renderer calling `openSubmenuPick` gets it for free. +**`submenu[].offSite`** — the row leaves the site. Off-site admin-menu entries are dropped server-side; the survivors are children of a plugin's own menu (a docs or account link), and they carry this flag. Nothing off-site can load in an iframe, so a surface that routes a URL into a window must skip them: the in-window tab strip does, and the constellation flyout marks them with an outbound glyph and hands them to the browser instead. `tryOpenExternalUrl()` is the shared escape; a renderer calling `openSubmenuPick` gets it for free. + +The name is `offSite` rather than `external` because the tab strip already spends that word on a different thing: a tab with `data-kind="external"` is a plugin-opened sub-iframe, which is on-site. **Lifecycle pairing — `replaceItems` ↔ `appendSystemItem`** — these are independent update paths. `replaceItems( items )` swaps the menu-derived tiles wholesale (the live menu refresh fires it on every plugin activation / deactivation). `appendSystemItem` / `removeSystemItem` track the JS-owned cohort (OpenStation Preferences, plugin native-window launchers). diff --git a/docs/plugin-compat-layer.md b/docs/plugin-compat-layer.md index c8d70969a..697b5d943 100644 --- a/docs/plugin-compat-layer.md +++ b/docs/plugin-compat-layer.md @@ -127,13 +127,13 @@ A site can be hosted somewhere that extends the admin menu with links to its own **Fix**, all in `openstation_build_dock_items()`: 1. Any menu whose resolved URL is off-site is dropped. `openstation_menu_item_is_external()` is the classifier, and it is filterable. -2. A child of a menu a regular plugin registered (`pluginFile` non-null) is the exception — a docs or account link under a plugin's own menu is a normal thing to ship. It keeps its row, flagged `external`, which the constellation marks with an outbound glyph and the in-window tab strip skips. +2. A child of a menu a regular plugin registered (`pluginFile` non-null) is the exception — a docs or account link under a plugin's own menu is a normal thing to ship. It keeps its row, flagged `offSite`, which the constellation marks with an outbound glyph and the in-window tab strip skips. 3. Rows carrying `hide-if-js` are dropped, which is also what removes the duplicate submenus WordPress.com produced: Jetpack keeps the wp-admin original and marks it hidden rather than replacing it, so the dock was rendering both copies. 4. Except when that hidden row is the original of an off-site row we just dropped — then it takes the dropped row's place in the list, and the menu opens the wp-admin screen Core registered. This is what puts Themes, Add Plugin and All Users back. **Icons**: `Base_Admin_Menu::override_svg_icons()` moves every SVG-data-URI menu icon into an inline stylesheet and sets `$menu[ $i ][6]` to `'none'`, which left Jetpack, MailPoet and every other plugin shipping vector art with a generic gear in the dock. `openstation_snapshot_menu_icons()` samples `$menu` at several points across `admin_menu` and records, write-once, the first real icon each slug wore; the builder falls back to it when the live value has been blanked. Sampling rather than parking one priority below the known rewriter is deliberate — registrations and rewrites both happen at arbitrary priorities, and the live value still wins whenever there is one, so a menu that genuinely changes its icon is unaffected. -Nothing here is WordPress.com-specific: the rules read the menu arrays, not the host. +Nothing here is WordPress.com-specific: the rules read the menu arrays, not the host. That cuts both ways, and rule 1 is the one to know about: a plugin that registers its **top-level** menu as a link to its own hosted service loses its dock tile on any site, where before it opened a browser tab. Children are the documented exception, top-level entries are not, because a tile that can never open a window is a tile that lies about what a dock click does. ### Empty submenu titles diff --git a/includes/core/payload.php b/includes/core/payload.php index 922bffde5..bf2abd1c2 100644 --- a/includes/core/payload.php +++ b/includes/core/payload.php @@ -181,6 +181,7 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) $rows[] = array( 'raw_title' => $sub_item[0], + 'slug' => (string) $sub_item[2], 'url' => $sub_url, 'external' => $sub_external, 'hidden' => openstation_menu_item_is_hidden( $sub_item ), @@ -226,10 +227,20 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) // self-link strip runs, so a restored original collapses into // `selfLabel` instead of becoming a child that duplicates its // own parent. + // + // Identity travels with it. Everything below keys off the menu's + // slug — whether it's a Core menu, whether a plugin owns it, + // whether it opens more than one window, and which slug the + // `openstation_dock_item` filter is told about. Left on the + // off-site slug, a rescued Plugins tile reads as a plugin menu + // owned by whoever registered the replacement, sorts to the far + // end of the dock, and offers to deactivate them. + $identity_slug = (string) $item[2]; if ( $parent_external ) { foreach ( $rows as $row ) { if ( ! $row['external'] ) { - $parent_url = $row['url']; + $parent_url = $row['url']; + $identity_slug = $row['slug']; break; } } @@ -312,7 +323,12 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) // Consumers that route a URL into a window skip these; // the ones that can hand a link to the browser mark // them as leaving the site. - $sub_entry['external'] = true; + // + // `offSite` rather than `external`: the window's tab + // strip already calls plugin-opened sub-iframe tabs + // "external" (`data-kind="external"`), and that is a + // different thing entirely. + $sub_entry['offSite'] = true; } $sub_items[] = $sub_entry; } @@ -365,10 +381,12 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) // named the way wp-admin names it. Empty when the menu had // no self-link to strip. 'selfLabel' => $self_label, - 'multi' => openstation_dock_item_is_multi( $item[2] ), - 'placement' => openstation_dock_placement( $item[2] ), - 'isCore' => openstation_is_core_menu_slug( $item[2] ), - 'pluginFile' => $plugin_file, + 'multi' => openstation_dock_item_is_multi( $identity_slug ), + 'placement' => openstation_dock_placement( $identity_slug ), + 'isCore' => openstation_is_core_menu_slug( $identity_slug ), + 'pluginFile' => $identity_slug === (string) $item[2] + ? $plugin_file + : openstation_resolve_menu_plugin_file( $identity_slug ), 'pluginName' => null, ); if ( $dock_item['pluginFile'] ) { @@ -381,7 +399,7 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) * @param array $dock_item The dock item data. * @param string $menu_slug The menu slug. */ - $dock_item = apply_filters( 'openstation_dock_item', $dock_item, $item[2] ); + $dock_item = apply_filters( 'openstation_dock_item', $dock_item, $identity_slug ); $items[] = $dock_item; } diff --git a/src/dock-constellation/index.ts b/src/dock-constellation/index.ts index 62da177eb..09fd835b3 100644 --- a/src/dock-constellation/index.ts +++ b/src/dock-constellation/index.ts @@ -1097,16 +1097,19 @@ function buildSubmenuRow( // An off-site row can't become a window — clicking it hands the URL // to the browser. Say so before the click, not after it. - if ( sub.external ) { + if ( sub.offSite ) { const mark = document.createElement( 'span' ); - mark.className = 'dashicons dashicons-external os-constellation__row-external'; + mark.className = 'dashicons dashicons-external os-constellation__row-offsite'; mark.setAttribute( 'aria-hidden', 'true' ); row.appendChild( mark ); - row.setAttribute( - 'aria-label', - // translators: %s is the submenu entry's label (e.g. "Documentation") - sprintf( __( '%s (opens in a new tab)' ), sub.title ), - ); + // Said in a hidden span rather than an `aria-label` on the row: + // a label REPLACES the accessible name, so it would drop the + // row's own text and anything a plugin appended to it. A span + // composes with them. + const note = document.createElement( 'span' ); + note.className = 'os-constellation__row-note'; + note.textContent = __( '(opens in a new tab)' ); + row.appendChild( note ); } row.addEventListener( 'click', () => { diff --git a/src/dock.ts b/src/dock.ts index 68c2a59e6..62bc34702 100644 --- a/src/dock.ts +++ b/src/dock.ts @@ -225,13 +225,17 @@ export interface SubmenuItem { /** * This row leaves the site. * - * Set server-side on a plugin menu's off-site children — a docs or + * Set server-side on a plugin menu's off-site children, a docs or * account link under the plugin's own menu. Nothing off-site can * load in a window, so surfaces that route a URL into one skip * these rows, and the ones that can hand a link to the browser mark * them as leaving. + * + * Named `offSite`, not `external`, because the window's tab strip + * already spends that word on something else: `data-kind="external"` + * is a plugin-opened sub-iframe tab, which is very much on-site. */ - external?: boolean; + offSite?: boolean; } export interface DockItem { @@ -270,7 +274,7 @@ export interface DockItem { /** Number badge (update count, comment count, etc.). 0 = no badge. */ badge: number; /** Submenu items. */ - submenu: { title: string; url: string; external?: boolean }[]; + submenu: { title: string; url: string; offSite?: boolean }[]; /** Whether this admin page supports multiple open windows. */ multi?: boolean; /** diff --git a/src/types.ts b/src/types.ts index f72489b67..11d2eb4f0 100644 --- a/src/types.ts +++ b/src/types.ts @@ -132,10 +132,10 @@ export interface WindowConfig { * Each tab navigates the iframe within the same window — no new window opens. * Pass an empty array (or omit) to hide the strip. * - * Rows flagged `external` are carried through but never become + * Rows flagged `offSite` are carried through but never become * tabs: the iframe can't load an off-site URL. */ - submenu?: { title: string; url: string; external?: boolean }[]; + submenu?: { title: string; url: string; offSite?: boolean }[]; /** * Optional initial state. When present, the window is constructed * into this state directly — used by session restore so a minimized @@ -1600,7 +1600,7 @@ export interface DockItemConfig { /** Badge count (updates, comments, etc.). */ badge: number; /** Submenu items. */ - submenu: { title: string; url: string; external?: boolean }[]; + submenu: { title: string; url: string; offSite?: boolean }[]; /** * WordPress's own label for this menu's landing page ("Themes", * "All Posts"), stripped out of `submenu` as the self-link. Names diff --git a/src/window/dom-external-tabs.test.ts b/src/window/dom-external-tabs.test.ts index 364f8cd7d..a399dde5a 100644 --- a/src/window/dom-external-tabs.test.ts +++ b/src/window/dom-external-tabs.test.ts @@ -48,7 +48,7 @@ describe( 'tab strip and off-site submenu rows', () => { test( 'an external row never becomes a tab', () => { const el = build( [ { title: 'Settings', url: ADMIN + 'admin.php?page=my-plugin-settings' }, - { title: 'Docs', url: 'https://example.org/docs', external: true }, + { title: 'Docs', url: 'https://example.org/docs', offSite: true }, ] ); expect( tabLabels( el ) ).toEqual( [ 'My Plugin', 'Settings' ] ); @@ -56,7 +56,7 @@ describe( 'tab strip and off-site submenu rows', () => { test( 'a submenu of nothing but external rows renders no strip', () => { const el = build( [ - { title: 'Docs', url: 'https://example.org/docs', external: true }, + { title: 'Docs', url: 'https://example.org/docs', offSite: true }, ] ); expect( tabLabels( el ) ).toEqual( [] ); diff --git a/src/window/dom.ts b/src/window/dom.ts index fe8c87ff1..310cc736a 100644 --- a/src/window/dom.ts +++ b/src/window/dom.ts @@ -852,7 +852,7 @@ export function createWindowElement( config: WindowConfig ): HTMLElement { // frame. They stay in the constellation flyout, which can hand // a link to the browser. const tabSubmenu = ( config.submenu ?? [] ).filter( - ( s ) => ! s.external, + ( s ) => ! s.offSite, ); if ( tabSubmenu.length > 0 && config.url ) { diff --git a/src/window/iframe-bridge.ts b/src/window/iframe-bridge.ts index 9890424ba..7785b6688 100644 --- a/src/window/iframe-bridge.ts +++ b/src/window/iframe-bridge.ts @@ -55,7 +55,7 @@ export interface AdminLinkDockEntry { * Optional — falls back to the destination URL when missing. */ url?: string; - submenu?: { title: string; url: string }[]; + submenu?: { title: string; url: string; offSite?: boolean }[]; multi?: boolean; } @@ -87,7 +87,7 @@ interface AdminLinkDispatchDeps { title: string; titleFromPage?: boolean; icon: string; - submenu?: { title: string; url: string }[]; + submenu?: { title: string; url: string; offSite?: boolean }[]; selfLabel?: string; multi?: boolean; } ): void; diff --git a/tests/phpunit/tests/openStationExternalMenuItems.php b/tests/phpunit/tests/openStationExternalMenuItems.php index ac26cb192..345b3f286 100644 --- a/tests/phpunit/tests/openStationExternalMenuItems.php +++ b/tests/phpunit/tests/openStationExternalMenuItems.php @@ -46,6 +46,7 @@ public function tear_down() { $icons = array(); remove_all_filters( 'openstation_menu_item_is_external' ); + remove_all_filters( 'openstation_dock_item' ); parent::tear_down(); } @@ -326,6 +327,53 @@ public function test_container_check_leaves_menus_without_off_site_children_alon $this->assertSame( admin_url( 'admin.php?page=empty-menu' ), $items[0]['url'] ); } + public function test_rescued_tile_takes_the_identity_of_the_slug_it_adopted() { + global $menu, $submenu; + // Identity has to move with the URL. Left on the off-site slug, + // this reads as a plugin menu owned by whoever registered the + // replacement, and sorts away from the Core tiles. + $this->attribute_to_plugin( 'https://wordpress.com/plugins/example.com', 'jetpack/jetpack.php' ); + + $menu = array( $this->make_menu_row( 'Plugins', 'activate_plugins', 'https://wordpress.com/plugins/example.com' ) ); + $submenu['https://wordpress.com/plugins/example.com'] = array( + array( 'Plugins', 'activate_plugins', 'plugins.php', '', 'hide-if-js' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertCount( 1, $items ); + $this->assertTrue( $items[0]['isCore'] ); + $this->assertNull( $items[0]['pluginFile'] ); + $this->assertNull( $items[0]['pluginName'] ); + } + + public function test_rescued_tile_reports_the_adopted_slug_to_the_dock_item_filter() { + global $menu, $submenu; + $seen = array(); + add_filter( + 'openstation_dock_item', + static function ( $dock_item, $menu_slug ) use ( &$seen ) { + $seen[] = $menu_slug; + return $dock_item; + }, + 10, + 2 + ); + + $menu = array( $this->make_menu_row( 'Plugins', 'activate_plugins', 'https://wordpress.com/plugins/example.com' ) ); + $submenu['https://wordpress.com/plugins/example.com'] = array( + array( 'Plugins', 'activate_plugins', 'plugins.php', '', 'hide-if-js' ), + ); + + try { + openstation_build_dock_items(); + } finally { + remove_all_filters( 'openstation_dock_item' ); + } + + $this->assertSame( array( 'plugins.php' ), $seen ); + } + public function test_hidden_top_level_menu_is_dropped() { global $menu; $menu = array( diff --git a/tests/vitest/dock-constellation.test.ts b/tests/vitest/dock-constellation.test.ts index 57e72e541..c52d9eb7b 100644 --- a/tests/vitest/dock-constellation.test.ts +++ b/tests/vitest/dock-constellation.test.ts @@ -334,7 +334,7 @@ describe( 'dock constellation', () => { { title: 'Docs', url: 'https://example.org/docs', - external: true, + offSite: true, }, ], }, @@ -343,11 +343,12 @@ describe( 'dock constellation', () => { const docs = rows( '.os-constellation__row--sub' )[ 1 ]; expect( - docs.querySelector( '.os-constellation__row-external' ), + docs.querySelector( '.os-constellation__row-offsite' ), ).not.toBeNull(); - expect( docs.getAttribute( 'aria-label' ) ).toBe( - 'Docs (opens in a new tab)', - ); + // Composed, not replaced: the row's own label survives. + expect( docs.getAttribute( 'aria-label' ) ).toBeNull(); + expect( docs.textContent ).toContain( 'Docs' ); + expect( docs.textContent ).toContain( '(opens in a new tab)' ); docs.click(); // A window would have loaded it into an iframe the remote From dbb7719fbd62e040a484744c1b5686842d91fa11 Mon Sep 17 00:00:00 2001 From: mmtr <1233880+mmtr@users.noreply.github.com> Date: Fri, 21 Aug 2026 14:50:48 +0200 Subject: [PATCH 3/4] Tests: Rename the tab-strip test file to match the offSite flag Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- .../{dom-external-tabs.test.ts => dom-offsite-tabs.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/window/{dom-external-tabs.test.ts => dom-offsite-tabs.test.ts} (100%) diff --git a/src/window/dom-external-tabs.test.ts b/src/window/dom-offsite-tabs.test.ts similarity index 100% rename from src/window/dom-external-tabs.test.ts rename to src/window/dom-offsite-tabs.test.ts From 1305d6b3f1509439847bf52a7ce755fb0a80ab90 Mon Sep 17 00:00:00 2001 From: mmtr <1233880+mmtr@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:05:48 +0200 Subject: [PATCH 4/4] Dock: fix the offSite key in tests, and keep an off-site menu alive by label or fallback Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- includes/core/payload.php | 31 ++++++++++ .../tests/openStationExternalMenuItems.php | 56 ++++++++++++++++++- 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/includes/core/payload.php b/includes/core/payload.php index bf2abd1c2..a98848c4a 100644 --- a/includes/core/payload.php +++ b/includes/core/payload.php @@ -209,10 +209,41 @@ function_exists( 'openstation_plugins_window_count_visible_updates' ) $rows[ $restore_slots[ $row_title ] ] = $row; $restored[ $row_title ] = true; } elseif ( $parent_external && $row_title === $title ) { + // The menu's own row, hidden in place. WordPress builds + // a parent's self-link by copying the menu row's first + // four fields, so its label is the menu's label, which + // is what makes the comparison hold. $keep[ $i ] = true; $restored[ $row_title ] = true; } } + + // Last resort for a menu whose own slug points off-site: if + // nothing on-site survived, take the first hidden on-site row + // rather than lose the menu. The label comparison above is the + // precise answer and covers the ordinary case, but it breaks the + // moment a host relabels the menu row without relabelling the + // self-link it already generated. Showing a row someone hid + // beats dropping a working menu off the dock. + if ( $parent_external ) { + $has_on_site = false; + foreach ( $rows as $i => $row ) { + if ( ! isset( $row['restore'] ) && $keep[ $i ] && ! $row['external'] ) { + $has_on_site = true; + break; + } + } + if ( ! $has_on_site ) { + foreach ( $rows as $i => $row ) { + if ( isset( $row['restore'] ) || ! $row['hidden'] || $row['external'] ) { + continue; + } + $keep[ $i ] = true; + break; + } + } + } + $kept_rows = array(); foreach ( $rows as $i => $row ) { if ( isset( $row['restore'] ) || ! $keep[ $i ] ) { diff --git a/tests/phpunit/tests/openStationExternalMenuItems.php b/tests/phpunit/tests/openStationExternalMenuItems.php index 345b3f286..1fb3507fe 100644 --- a/tests/phpunit/tests/openStationExternalMenuItems.php +++ b/tests/phpunit/tests/openStationExternalMenuItems.php @@ -128,10 +128,10 @@ public function test_off_site_submenu_of_a_plugin_menu_is_kept_and_flagged() { $this->assertCount( 1, $items[0]['submenu'] ); $this->assertSame( 'Help', $items[0]['submenu'][0]['title'] ); - $this->assertTrue( $items[0]['submenu'][0]['external'] ); + $this->assertTrue( $items[0]['submenu'][0]['offSite'] ); } - public function test_internal_submenu_is_not_flagged_external() { + public function test_internal_submenu_is_not_flagged_off_site() { global $menu, $submenu; $menu = array( $this->make_menu_row( 'Posts', 'edit_posts', 'edit.php' ) ); $submenu['edit.php'] = array( @@ -141,7 +141,7 @@ public function test_internal_submenu_is_not_flagged_external() { $items = openstation_build_dock_items(); - $this->assertArrayNotHasKey( 'external', $items[0]['submenu'][0] ); + $this->assertArrayNotHasKey( 'offSite', $items[0]['submenu'][0] ); } public function test_off_site_submenu_never_becomes_the_parent_url() { @@ -374,6 +374,56 @@ static function ( $dock_item, $menu_slug ) use ( &$seen ) { $this->assertSame( array( 'plugins.php' ), $seen ); } + public function test_off_site_parent_restores_its_own_row_by_label() { + global $menu, $submenu; + $menu = array( $this->make_menu_row( 'Plugins', 'activate_plugins', 'https://wordpress.com/plugins/example.com' ) ); + $submenu['https://wordpress.com/plugins/example.com'] = array( + array( 'Plugins', 'activate_plugins', 'plugins.php', '', 'hide-if-js' ), + array( 'Plugin File Editor', 'edit_plugins', 'plugin-editor.php', '', 'hide-if-js' ), + ); + + $items = openstation_build_dock_items(); + + // The label match picks the menu's own row, not whichever hidden + // row happens to come first. + $this->assertCount( 1, $items ); + $this->assertSame( admin_url( 'plugins.php' ), $items[0]['url'] ); + $this->assertSame( array(), $items[0]['submenu'] ); + } + + public function test_off_site_parent_falls_back_to_any_hidden_on_site_row() { + global $menu, $submenu; + // A host that relabelled the menu row without relabelling the + // self-link it had already generated. Nothing matches by label, + // and dropping the menu would lose a page that works. + $menu = array( $this->make_menu_row( 'Manage plugins', 'activate_plugins', 'https://wordpress.com/plugins/example.com' ) ); + $submenu['https://wordpress.com/plugins/example.com'] = array( + array( 'Plugins', 'activate_plugins', 'plugins.php', '', 'hide-if-js' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertCount( 1, $items ); + $this->assertSame( admin_url( 'plugins.php' ), $items[0]['url'] ); + $this->assertTrue( $items[0]['isCore'] ); + } + + public function test_off_site_parent_prefers_an_on_site_child_over_a_hidden_row() { + global $menu, $submenu; + // A visible on-site child is a better stand-in than a row + // someone hid, so the fallback stays out of the way. + $menu = array( $this->make_menu_row( 'Manage plugins', 'activate_plugins', 'https://wordpress.com/plugins/example.com' ) ); + $submenu['https://wordpress.com/plugins/example.com'] = array( + array( 'Plugins', 'activate_plugins', 'plugins.php', '', 'hide-if-js' ), + array( 'Add Plugin', 'install_plugins', 'plugin-install.php' ), + ); + + $items = openstation_build_dock_items(); + + $this->assertCount( 1, $items ); + $this->assertSame( admin_url( 'plugin-install.php' ), $items[0]['url'] ); + } + public function test_hidden_top_level_menu_is_dropped() { global $menu; $menu = array(