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.

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

Fix deprecation warning on AMP validation post edit screen #7651

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

thelovekesh
Copy link
Collaborator

@thelovekesh thelovekesh commented Nov 1, 2023

Summary

On a post-edit screen, the title is determined using the edit_item label in the post object and post title which returns a string for the title like Edit post "Post title".

In our case, edit_item is being set as an empty string which causes a deprecation warning with the PHP 8.1+ version.

image

Since we want to stop the passing null deprecation warning, this PR is a space in the string('' -> ' ') that stops the warning.

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@thelovekesh thelovekesh added this to the v2.5.0 milestone Nov 1, 2023
Copy link
Contributor

github-actions bot commented Nov 1, 2023

Plugin builds for 68745e4 are ready 🛎️!

Checksums
# Development build checksums
f90a66829c58b0cfd1e853deb0c0d9f49ab0882c17c0f2b39d3a46fff44262d1 *amp.zip

# Production build checksums
76411b709620f2732f3ab3f2e1353343e8658c50488ed5dba5a50446c889c64f *amp.zip

Warning

These builds are for testing purposes only and should not be used in production.

@westonruter
Copy link
Member

Interesting, as '' is an empty string and not null, right?

@westonruter
Copy link
Member

And passing '' as the 3rd arg to preg_replace() doesn't cause a deprecation warning: https://3v4l.org/IerE4

Although passing null does: https://3v4l.org/vFJJS

@westonruter
Copy link
Member

In short, I don't see how the change should fix the issue.

@thelovekesh
Copy link
Collaborator Author

It short-circuits the logic to determine the title when the title is empty.

https://github.com/WordPress/wordpress-develop/blob/126d780de939cb4036e53cd8abe705426767661c/src/wp-admin/includes/plugin.php#L1976

We are already filtering title so a fallback isn't required anyway

public static function filter_admin_title( $admin_title ) {

@westonruter westonruter merged commit 1a0c049 into develop Nov 1, 2023
34 checks passed
@westonruter westonruter deleted the fix/deprecation-warning branch November 1, 2023 20:09
@pavanpatil1
Copy link

QA Passed ✅

Cross-checked the issue and it is working fine. The deprecation warning is not visible now.

Before fix After fix
image image

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants