Make WordPress Core

Opened 4 years ago

Closed 7 weeks ago

#56607 closed defect (bug) (fixed)

Improve doc for WP_Screen class

Reported by: Chouby Owned by: SergeyBiryukov
Priority: normal Milestone: 7.1
Component: Administration Version: 3.3
Severity: normal Keywords: has-patch
Cc: Focuses: docs

Description

WooCommerce 6.9 introduced a conflict with several plugins by accessing to the property WP_Screen::id and expecting it to be a string, while conflicting plugins just set the current screen with a simple call to set_current_screen() without parameter. In this case, get_current_screen() returns a WP_Screen object with the property id set to null.

This pushed me to review the doc of the class and notice that several properties are not correctly documented for this case.

Attachments (1)

56607.patch (1.4 KB ) - added by Chouby 4 years ago.

Download all attachments as: .zip

Change History (5)

@Chouby
4 years ago

#1 @Chouby
4 years ago

  • Keywords has-patch added

Just for reference, the issue opened in WooCommerce: https://github.com/woocommerce/woocommerce/issues/34725

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


7 weeks ago
#2

## Summary

  • Correct the documented type of WP_Screen::$_screen_settings.
  • Account for the property being null until show_screen_options() initializes it.

The ticket's other proposed type changes have already been addressed by later commits or are no longer applicable to current trunk.

## Testing

  • php -l src/wp-admin/includes/class-wp-screen.php
  • phpcbf --standard=phpcs.xml.dist src/wp-admin/includes/class-wp-screen.php
  • phpcs --standard=phpcs.xml.dist src/wp-admin/includes/class-wp-screen.php

#3 in reply to: ↑ description @SergeyBiryukov
7 weeks ago

  • Milestone Awaiting Review7.1

Replying to Chouby:

...accessing to the property WP_Screen::id and expecting it to be a string, while conflicting plugins just set the current screen with a simple call to set_current_screen() without parameter. In this case, get_current_screen() returns a WP_Screen object with the property id set to null.

Thanks for the ticket! Just noting that I cannot reproduce this specific issue in current trunk:

require 'wp-load.php';
require 'wp-admin/includes/admin.php';

set_current_screen();
var_dump( get_current_screen() );

Both WP_Screen::$id and WP_Screen::$base are an empty string in this scenario in my testing.

That said, WP_Screen::$_screen_settings is indeed null and appears to be the only property left to correct here, as the other affected properties have already been adjusted in [55693] and [61300].

#4 @SergeyBiryukov
7 weeks ago

  • Owner set to SergeyBiryukov
  • Resolutionfixed
  • Status newclosed

In 62837:

Docs: Correct the type for WP_Screen::$_screen_settings.

This reflects the property's initial null state prior to initialization.

Follow-up to [55693], [61300].

Props Chouby, arkaprabhachowdhury, SergeyBiryukov.
Fixes #56607.

Note: See TracTickets for help on using tickets.