Make WordPress Core

Opened 3 months ago

Closed 3 months ago

#63267 closed defect (bug) (fixed)

Coding Standards: Check for an empty $old_email first in wp_site_admin_email_change_notification()

Reported by: dilipbheda's profile dilipbheda Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.9 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: coding-standards Cc:

Description

In the wp_site_admin_email_change_notification() function, the current logic checks the $old_email variable for a specific default value before checking if it's empty:

<?php
if ( 'you@example.com' === $old_email || empty( $old_email ) ) {
        $send = false;
}

This should be reordered to first check if $old_email is empty:

<?php
if ( empty( $old_email ) || 'you@example.com' === $old_email ) {
        $send = false;
}

This change follows a common best practice: always check for empty or null values first to avoid unnecessary comparisons or potential errors. It also aligns better with readability and expected evaluation flow.

Change History (9)

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


3 months ago
#1

#2 @Presskopp
3 months ago

How comes the code is different on https://developer.wordpress.org/reference/functions/wp_site_admin_email_change_notification/ ?

if ( 'you@example.com' === $old_email ) {
    $send = false;
}

#4 @dilipbheda
3 months ago

I think it changed 7 days ago with this [GitHub issue]https://github.com/dilipbheda/wordpress-develop/issues/62211, but I can't access it to review the changes.

Ref: https://tinyurl.com/2yk94pdn

Last edited 3 months ago by dilipbheda (previous) (diff)

#6 @dilipbheda
3 months ago

@Presskopp Thank you for the sharing; I've updated the multisite function as well in my opened PR.

#7 @dilipbheda
3 months ago

Related to #62211

Last edited 3 months ago by SergeyBiryukov (previous) (diff)

#8 @SergeyBiryukov
3 months ago

  • Milestone changed from Awaiting Review to 6.9

#9 @SergeyBiryukov
3 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 60153:

Coding Standards: Check for an empty address first on admin email change notification.

This follows a common best practice of checking for an empty value before doing a specific comparison.

Follow-up to [60122], [60129].

Props dilipbheda, Presskopp.
Fixes #63267.

Note: See TracTickets for help on using tickets.