Skip to content

Commit 90b7ea7

Browse files
arraypressclaude
andcommitted
Fix the page URL when the parent is a top-level menu
page_url() passed parent_slug straight to admin_url(), which is only right when the page hangs off one of core's menus and the slug is a filename -- options-general.php, edit.php. A plugin with its own top-level menu passes that menu's slug instead, and admin_url( 'my-plugin' ) builds /wp-admin/my-plugin?page=..., which is not an admin screen: WordPress serves the front end from it. Every tab link and every redirect after saving therefore left the admin entirely, on any plugin registering its settings under its own menu. register_menu() was already correct -- add_submenu_page() handles a non-file parent by putting the page at admin.php?page=... -- so the fix is for page_url() to resolve the same way. The regression test reaches page_url() by reflection: both callers, the tab links and the redirect, are private, and the bug is in the URL rather than in either of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tk8y5eZa3CHVVSM6nVxsJw
1 parent 9debca7 commit 90b7ea7

2 files changed

Lines changed: 108 additions & 1 deletion

File tree

‎src/SettingFields.php‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -800,9 +800,22 @@ private function tab_links(): array {
800800
* @return string
801801
*/
802802
private function page_url( array $args = [] ): string {
803+
$parent = (string) $this->config['parent_slug'];
804+
805+
// A parent slug is only a filename when the page hangs off one of
806+
// core's menus -- options-general.php, edit.php. A plugin with its
807+
// own top-level menu passes that menu's slug instead, and its
808+
// children live at admin.php?page=..., exactly as add_submenu_page()
809+
// registers them a few methods up. Passing the slug to admin_url()
810+
// builds /wp-admin/my-plugin?page=..., which is not an admin screen
811+
// at all -- WordPress serves the front end from it.
812+
$base = ( '' !== $parent && str_ends_with( $parent, '.php' ) )
813+
? $parent
814+
: 'admin.php';
815+
803816
return add_query_arg(
804817
array_merge( [ 'page' => (string) $this->config['menu_slug'] ], array_filter( $args ) ),
805-
admin_url( '' !== (string) $this->config['parent_slug'] ? (string) $this->config['parent_slug'] : 'admin.php' )
818+
admin_url( $base )
806819
);
807820
}
808821

‎tests/PageUrlTest.php‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
<?php
2+
/**
3+
* The URL a settings screen links to itself with.
4+
*
5+
* @package ArrayPress\RegisterSettingFields
6+
*/
7+
8+
declare( strict_types=1 );
9+
10+
namespace ArrayPress\RegisterSettingFields\Tests;
11+
12+
use ArrayPress\RegisterSettingFields\Registry;
13+
use ArrayPress\RegisterSettingFields\SettingFields;
14+
use PHPUnit\Framework\TestCase;
15+
use ReflectionMethod;
16+
17+
/**
18+
* A parent slug is only a filename when the page hangs off one of core's
19+
* menus. A plugin with its own top-level menu passes that menu's slug, and
20+
* passing that to admin_url() builds /wp-admin/my-plugin?page=... -- which is
21+
* not an admin screen at all, so every tab link and every post-save redirect
22+
* lands on the front end.
23+
*
24+
* Reached by reflection because both callers -- the tab links and the
25+
* redirect after saving -- are private, and the bug is in the URL rather than
26+
* in either of them.
27+
*/
28+
final class PageUrlTest extends TestCase {
29+
30+
protected function tearDown(): void {
31+
foreach ( array_keys( Registry::instance()->all() ) as $id ) {
32+
Registry::instance()->unregister( (string) $id );
33+
}
34+
35+
parent::tearDown();
36+
}
37+
38+
/**
39+
* Build a page and read the URL it would link to.
40+
*
41+
* @param string $parent_slug What the page hangs off.
42+
* @param array $args Extra query arguments.
43+
*
44+
* @return string
45+
*/
46+
private function url( string $parent_slug, array $args = [] ): string {
47+
$page = new SettingFields(
48+
'sf_url_' . md5( $parent_slug ),
49+
[
50+
'option_name' => 'sf_url',
51+
'menu_slug' => 'my-plugin-settings',
52+
'parent_slug' => $parent_slug,
53+
'tabs' => [ 'one' => 'One' ],
54+
'fields' => [ 'first' => [ 'type' => 'text', 'tab' => 'one' ] ],
55+
]
56+
);
57+
58+
// No setAccessible(): private methods have been reflectively
59+
// callable without it since PHP 8.1, and the call is deprecated.
60+
return (string) ( new ReflectionMethod( $page, 'page_url' ) )->invoke( $page, $args );
61+
}
62+
63+
public function test_a_top_level_menu_parent_resolves_to_admin_php(): void {
64+
$url = $this->url( 'my-plugin' );
65+
66+
$this->assertStringContainsString(
67+
'admin.php',
68+
$url,
69+
'A parent that is not a file must resolve to admin.php, where add_submenu_page() puts it.'
70+
);
71+
$this->assertStringNotContainsString(
72+
'/wp-admin/my-plugin',
73+
$url,
74+
'The parent slug was used as a path, which is a front end URL.'
75+
);
76+
$this->assertStringContainsString( 'page=my-plugin-settings', $url );
77+
}
78+
79+
public function test_a_core_menu_parent_keeps_its_filename(): void {
80+
$this->assertStringContainsString( 'options-general.php', $this->url( 'options-general.php' ) );
81+
$this->assertStringContainsString( 'edit.php', $this->url( 'edit.php' ) );
82+
}
83+
84+
public function test_no_parent_resolves_to_admin_php(): void {
85+
$this->assertStringContainsString( 'admin.php', $this->url( '' ) );
86+
}
87+
88+
public function test_extra_arguments_survive(): void {
89+
$url = $this->url( 'my-plugin', [ 'tab' => 'payments' ] );
90+
91+
$this->assertStringContainsString( 'tab=payments', $url );
92+
$this->assertStringContainsString( 'page=my-plugin-settings', $url );
93+
}
94+
}

0 commit comments

Comments
 (0)