Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WPcom_Admin_Menu: Add special cases that don't have menus extended #22804

Closed
wants to merge 7 commits into from
Closed

WPcom_Admin_Menu: Add special cases that don't have menus extended #22804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e62ff1c
[not verified] Base_Admin_Menu: Don't add admin_menu actions for /wp-…
mreishus Feb 10, 2022
b6d0386
[not verified] Add full-stop to end of comment
mreishus Feb 10, 2022
9a1811a
[not verified] Move special cases to should_skip_menu_processing() fu…
mreishus Feb 11, 2022
dbfbee3
[not verified] Move admin menu customization bypass to load.php
mreishus Feb 11, 2022
36d604d
[not verified] Add periods to the ends of comments
mreishus Feb 11, 2022
816246c
[not verified] The ?service-worker mechanism does not provide a nonce…
mreishus Feb 11, 2022
316b66c
[not verified] Add changelog
mreishus Feb 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/wpcom-admin-menu-bypass-special-cases
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: other

WPCOM: Admin Menu customizations bypassed on files that do not render a menu
15 changes: 15 additions & 0 deletions projects/plugins/jetpack/modules/masterbar/admin-menu/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,21 @@ function should_customize_nav( $admin_menu_class ) {
return false;
}

// WPCOM-specific exemptions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently maintain the Nav Unification activation logic for Simple sites in the wpcom codebase (in the show_unified_nav function which lives in wp-content/mu-plugins/masterbar.php), so I guess it makes sense to also add these new checks there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some testing (the patch is in D75746-code). It looks like it's another way to accomplish the same thing, either this PR or D75746-code is needed, but no extra benefit from both. Maybe D75746-code is better because it keeps the wpcom specific exemptions in wpcom and out of jetpack? I'd be fine w/ closing this PR and going for D75746-code.

2022-02-25_15-07
2022-02-25_15-08
2022-02-25_15-09
2022-02-25_15-10

Copy link
Member

@obenland obenland Feb 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think keeping wpcom's special case on wpcom in D75746-code makes sense. With WordPress/gutenberg#38810 about to be merged you could even keep it to just the *stuff.php file checks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be fine w/ closing this PR and going for D75746-code.

Let's do that then 🙂

if ( is_a( $admin_menu_class, WPcom_Admin_Menu::class, true ) && ! $is_api_request ) {
$script_filename = ( ! empty( $_SERVER['SCRIPT_FILENAME'] ) ) ? basename( $_SERVER['SCRIPT_FILENAME'] ) : null;

// Special case: /wp-admin/?service-worker doesn't render a menu at all: skip all menu work.
if ( 'index.php' === $script_filename && isset( $_GET['service-worker'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
return false;
}

// Special case: index-yourstuff.php and index-hotstuff.php don't render menus.
if ( 'index-yourstuff.php' === $script_filename || 'index-hotstuff.php' === $script_filename ) {
return false;
}
}

return true;
}

Expand Down