Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#57777 closed defect (bug) (fixed)

Twenty Twenty: Deprecation warnings on PHP 8.1 because of `wp_add_inline_style()`

Reported by: swissspidy's profile swissspidy Owned by: flixos90's profile flixos90
Milestone: 6.2 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch has-testing-info commit
Focuses: Cc:

Description

Currently running 6.2-beta2-55368 on PHP 8.1 and getting the following deprecation notice when opening the block editor:

Deprecated: stripos(): Passing null to parameter #1 ($haystack) of type string is deprecated in /../wp-includes/functions.wp-styles.php

This is caused by the twentytwenty_block_editor_styles() function which calls

wp_add_inline_style( $handle = 'twentytwenty-block-editor-styles', $data = NULL ).

I see two problematic calls, though the latter one :

https://github.com/WordPress/wordpress-develop/blob/072274690b52cfa0c5e5da763e70a7af24f00da7/src/wp-content/themes/twentytwenty/functions.php#L426-L430

  • twentytwenty_get_customizer_css() can return false - not exactly a valid param to wp_add_inline_style
  • TwentyTwenty_Non_Latin_Languages::get_non_latin_css() returns void <- this is the more severe issue as this means $data is null

Both instances should be fixed imo.

Change History (9)

#1 @flixos90
2 years ago

@swissspidy I know this isn't the main concern here, but can you clarify in which circumstances twentytwenty_get_customizer_css() could return false? I fail to see it when looking at the code. Wouldn't ob_get_clean() always return a string as long as there is an output buffer (which there will be given the unconditional ob_start() call before)?

This ticket was mentioned in PR #4123 on WordPress/wordpress-develop by @flixos90.


2 years ago
#2

  • Keywords has-patch added
  • Fixes the bug outlined in the ticket.
  • Also fixes inaccurate @return docs and makes the null return explicit.

Trac ticket: https://core.trac.wordpress.org/ticket/57777

#3 @swissspidy
2 years ago

@flixos90 not at my computer right now, so that return type is purely based on what static analysis / IDE provides. Since intput buffers could be modified by something else, I wouldn‘t rely on a string return type personally

#4 @sabernhardt
2 years ago

These functions are pluggable, so you technically could have anything in the output.

I have not set up a site with PHP 8.1 to test my patch (draft), but the conditional statements could be more thorough and check all three instances of each function.

Last edited 2 years ago by sabernhardt (previous) (diff)

#5 @poena
2 years ago

Test Report

This report validates that the indicated patch addresses the first reproducable issue with the deprecated notice.

Patch tested: PR #4123

Environment

  • OS: macOS 13.2.1
  • Web Server: Nginx
  • PHP: 8.1.9
  • WordPress: 6.2-beta3-55409
  • Browser: Chrome 110.0.5481.100
  • Theme: Twenty Twenty
  • Active Plugins:
    • WordPress Beta Tester 3.2.7

Actual Results

  • ✅ Issue resolved with patch.

#6 @mukesh27
2 years ago

  • Keywords has-testing-info commit added
  • Owner set to flixos90
  • Status changed from new to assigned

Thanks @swissspidy for the ticket!

@flixos90 I reviewed the PR #4123 and changes look good to me.

Thanks @poena for testing report.

Add commit keyword for consideration.

#7 @flixos90
2 years ago

@swissspidy @sabernhardt Fair points, I have pushed a new commit https://github.com/WordPress/wordpress-develop/pull/4123/commits/7f43079ac58ecd4fcdf97460704c47241c0393c0 that also avoids adding the Customizer styles if they're empty or false-y.

Thank you @poena and @mukesh27 for the testing and review!

#8 @flixos90
2 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 55427:

Twenty Twenty: Avoid PHP warnings in 8.1 due to incorrect usage of wp_add_inline_style().

This changeset adjusts the logic to use a more defensive approach, to avoid passing potentially invalid values to wp_add_inline_style().

Props flixos90, mukesh27, costdev, swissspidy, poena, sabernhardt.
Fixes #57777.

Note: See TracTickets for help on using tickets.