Opened 6 months ago
Last modified 6 months ago
#63621 new enhancement
Enhancement: Ensure Hello Dolly passess the Plugin Check (PCP)
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Awaiting Review | Priority: | normal |
| Severity: | minor | Version: | |
| Component: | Plugins | Keywords: | has-patch has-test-info has-screenshots |
| Focuses: | coding-standards | Cc: |
Description
This ticket details several issues found in the Hello Dolly plugin, along with proposed solutions to ensure it passes the Plugin Check (PCP).
See screenshot when I tested it using Plugin Check. 
And here is the screenshot after I applied the changes below: 
Issue 1: Missing Text Domain in () Function
Problem: The () function on line 67 is missing the required $domain parameter for internationalization, which is a PCP requirement.
Fix: Add 'hello-dolly' as the text domain.
// Original: __( 'Quote from Hello Dolly song, by Jerry Herman:' ),
// Fixed: __( 'Quote from Hello Dolly song, by Jerry Herman:', 'hello-dolly' ),
Issue 2: Missing License Information in Plugin Header
Problem: The plugin header is missing "License" and "License URI" information, which is a standard PCP check.
Fix: Add the following lines to the plugin header, ideally before Author: Matt Mullenweg.
/* Plugin Name: Hello Dolly Plugin URI: http://wordpress.org/plugins/hello-dolly/ Description: This is not just a plugin, it symbolizes the hope and enthusiasm of an entire generation summed up in two words sung most famously by Louis Armstrong: Hello, Dolly. When activated you will randomly see a lyric from <cite>Hello, Dolly</cite> in the upper right of your admin screen on every page. Author: Matt Mullenweg Version: 1.7.2 License: GPLv2 or later License URI: https://www.gnu.org/licenses/gpl-2.0.html Author URI: http://ma.tt/ */
Issue 3: Unescaped Output
Problem: On lines 67, 68, and 69, output is not being properly escaped, leading to potential Cross-Site Scripting (XSS) vulnerabilities. This is a critical security check in PCP.
Fix: Apply appropriate escaping functions:
For the translatable string on line 67, use esc_html().
For $lang on line 68 (used within an HTML attribute), use esc_attr().
For $chosen on line 69 (plain text output), use esc_html().
// Original:
printf(
'<p id="dolly"><span class="screen-reader-text">%s </span><span dir="ltr"%s>%s</span></p>',
__( 'Quote from Hello Dolly song, by Jerry Herman:' ), // Line 67
$lang, // Line 68
$chosen // Line 69
);
// Fixed:
printf(
'<p id="dolly"><span class="screen-reader-text">%s </span><span dir="ltr"%s>%s</span></p>',
esc_html__( 'Quote from Hello Dolly song, by Jerry Herman:', 'hello-dolly' ), // Line 67
esc_attr( $lang ), // Line 68
esc_html( $chosen ) // Line 69
);
Issue 4: Discouraged mt_rand() Function
Problem: On line 54, mt_rand() is used, which is discouraged in WordPress for consistency and better randomness, as highlighted by PCP.
Fix: Replace mt_rand() with wp_rand().
// Original: return wptexturize( $lyrics[ mt_rand( 0, count( $lyrics ) - 1 ) ] );
// Fixed: return wptexturize( $lyrics[ wp_rand( 0, count( $lyrics ) - 1 ) ] );
Attachments (1)
Change History (4)
This ticket was mentioned in PR #9081 on WordPress/wordpress-develop by @jhimross.
6 months ago
#1
This PR addresses several identified issues in the hello.php file of the Hello Dolly plugin to improve code quality, security, and ensure that it passes the Plugin Check (PCP).
Key Changes:
1.Internationalization (i18n) Improvements:
Added the correct text domain (hello-dolly) to the () function call on line 67, resolving the Missing $domain parameter error.
Note on TextDomainMismatch warning: While a linter might report TextDomainMismatch on line 69 expecting plugins, hello-dolly is the correct and specific text domain for this plugin. The linter warning has been suppressed using phpcs:ignore WordPress.WP.I18n.TextDomainMismatch.
2.Plugin Header Compliance:
Added License: GPLv2 or later and License URI: https://www.gnu.org/licenses/gpl-2.0.html to the plugin header, resolving the Missing "License" in Plugin Header error.
3.Output Escaping for Security:
Implemented proper escaping for all output printed to the screen on lines 67, 68, and 69 to prevent Cross-Site Scripting (XSS) vulnerabilities.
Used esc_html() for translatable HTML content.
Used esc_attr() for HTML attribute values.
Used esc_html() for general text output.
4.Random Number Generation Best Practice:
Replaced mt_rand() with wp_rand() on line 54 for improved randomness and adherence to WordPress's recommended functions.
Trac ticket:
---
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.
WordPress Trac Ticket: https://core.trac.wordpress.org/ticket/63621
This ticket was mentioned in Slack in #core-test by oglekler. View the logs.
6 months ago
#3
@
6 months ago
- Focuses coding-standards added
updated hello.php with the patch