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

Jump to bottom

Fix PHP warnings coming from Classic block compat experiment #52967

Closed
wants to merge 1 commit into from
Closed

Fix PHP warnings coming from Classic block compat experiment #52967

wants to merge 1 commit into from

Conversation

Mamaduka
Copy link
Member

@Mamaduka Mamaduka commented Jul 26, 2023 •
edited
Loading

What?

PR updates early bailout logic in gutenberg_post_being_edited_requires_classic_block to avoid PHP notices when visiting admin pages.

PHP Warning:  Undefined variable $current_post in ~/gutenberg/wp-content/plugins/gutenberg/lib/experimental/disable-tinymce.php on line 77
PHP Warning:  Attempt to read property "post_type" on null in ~/gutenberg/wp-content/plugins/gutenberg/lib/experimental/disable-tinymce.php on line 77

Why?

The previous early return condition was never reached without post and action query args.

How?

Testing Instructions

  1. Disable TinyMCE and Classic block from Gutenberg > Experiments.
  2. Refresh the experiments page or go to the dashboard.
  3. Confirm no PHP errors are logged.

@Mamaduka Mamaduka requested a review from spacedmonkey as a code owner July 26, 2023 07:45
Mamaduka
Mamaduka commented Jul 26, 2023
View reviewed changes
lib/experimental/disable-tinymce.php

$content = $current_post->post_content;
if ( ! $current_post ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the WP_Error check; the get_post returns null on failure.

@github-actions
Copy link

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files
❔ lib/experimental/disable-tinymce.php

Mamaduka
Mamaduka commented Jul 26, 2023
View reviewed changes
lib/experimental/disable-tinymce.php
if ( ! $current_post || is_wp_error( $current_post ) ) {
return false;
}
$current_post = get_post( absint( $_GET['post'] ) );
Copy link
Member Author

Choose a reason for hiding this comment

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

Nit: The post IDs should be non-negative integers.

@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Block] Classic Affects the Classic Editor Block [Type] Experimental Experimental feature or API. labels Jul 26, 2023
@Mamaduka Mamaduka self-assigned this Jul 26, 2023
@Mamaduka Mamaduka requested review from tyxla and removed request for spacedmonkey July 26, 2023 07:52
@Mamaduka Mamaduka added the [Status] Duplicate Used to indicate that a current issue matches an existing one and can be closed label Jul 26, 2023
@Mamaduka
Copy link
Member Author

Closing in favor of #52935, which I missed.

@Mamaduka Mamaduka closed this Jul 26, 2023
@Mamaduka Mamaduka deleted the fix/classic-block-compat-warnings branch July 26, 2023 07:56
@tyxla
Copy link
Member

tyxla commented Jul 26, 2023

Oops! Thanks for reviewing #52935 and for going so quickly to file a fix, @Mamaduka 🙌

@github-actions
Copy link

Flaky tests detected in c99ea15.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5666060181
📝 Reported issues:

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

@tyxla tyxla Awaiting requested review from tyxla

Assignees

@Mamaduka Mamaduka

Labels
[Block] Classic Affects the Classic Editor Block [Status] Duplicate Used to indicate that a current issue matches an existing one and can be closed [Type] Bug An existing feature does not function as intended [Type] Experimental Experimental feature or API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Mamaduka @tyxla