WordPress.org

Make WordPress Core

Opened 3 weeks ago

Closed 12 days ago

Last modified 10 days ago

#50413 closed defect (bug) (fixed)

Update code and comments to remove "blacklist" and "whitelist"

Reported by: strangerstudios Owned by: desrosj
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch needs-dev-note
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Specifically, we want to change any use of "blacklist" to "blocklist" and any use of "whitelist" to "safelist".

In some cases a different word may be more appropriate or the language can be changed to avoid the issue.

I spent some time finding all of the cases of blacklist and whitelist in the core WP code and took notes on what actions could be taken.

Sometimes we can just change the words with no downside (that I could see). Sometimes the word use is based on a dependency in an included library. Sometimes the use is in a default theme. Sometimes we need to do more work to deprecate old function and filter names.

Here are my all my notes. I still need to do more research on uses of whitelist in particular.

I think we should break things down into smaller chunks. The "easy" cases can be separated out. And some of the cases are related around a common WP feature.

  • START NOTES -

Older trac ticket that updated the language on the front end: #48900

Here are the instances where the term blacklist shows up in the code.

https://github.com/WordPress/wordpress-develop/search?q=blacklist&unscoped_q=blacklist

A) The password-strengh-meter.js file uses blacklist
https://github.com/WordPress/WordPress/blob/2a0489ec49c449894471a599c1811aaf0fcbbb22/wp-admin/js/password-strength-meter.js

The blocklist functionality is only in our WP code. The zxcvbn library doesn't have a blocklist.

  1. Change the method and variable names in this file.
  1. This other WP core file uses the old method name: https://github.com/WordPress/WordPress/blob/001ffe81fbec4438a9f594f330e18103d21fbcd7/wp-admin/js/user-profile.js#L223
  1. We need to check if other plugins are using the specific userInputBlacklist method. At least one use here: https://github.com/strangerstudios/paid-memberships-pro/blob/272b47cf1cf041a45734678dfbf15b3ca699a241/js/pmpro-login.js#L15
  1. We potentially need to keep a version of the old method that is deprecated.
  1. Change the use in this unit test: https://github.com/WordPress/wordpress-develop/blob/98584d6e1191ad7ed62f79c13c343dade456d95d/tests/qunit/wp-admin/js/password-strength-meter.js

---

B) The sodium_compat library uses the term in its code. https://github.com/WordPress/WordPress/blob/d4ef90b236ce340591bd284b2ecaa085e4f42ffd/wp-includes/sodium_compat/src/Core/Ed25519.php#L380

  1. We should ask the maintainers of that library to consider changing their code to use the more inclusive terms. I believe this is the repo https://github.com/paragonie/sodium_compat. I didn't find any existing issues or PRs that address this. We could create an issue and/or PR for them.

---

C) The twentynineteen npm package.json has a config options for rtlccssConfig called blacklist. The blacklist value is set to {}. Based on this doc (https://rtlcss.com/learn/usage-guide/options/index.html) the default is {} and unlikely to change IMO. There is another option "useCalc" that isn't set in our package.json.

  1. We can remove this line https://github.com/WordPress/WordPress/blob/9e9be350e2b64d5c680219608b346e606f2223dc/wp-content/themes/twentynineteen/package.json#L23 and nothing should break.

---

D) The twentytwenty npm package.json has the same line as above.

  1. We can remove this line https://github.com/WordPress/WordPress/blob/9e9be350e2b64d5c680219608b346e606f2223dc/wp-content/themes/twentytwenty/package.json#L36 and nothing should break.

---

E) There is a function called wp_blacklist_check() which checks if a comment contains blocklisted characters or words. https://github.com/WordPress/wordpress-develop/blob/3bdf8b7b020be9b3b2308a4fdf6a27d5dde15dc4/src/wp-includes/comment.php#L1277

  1. There's also a hook named wp_blacklist_check. https://github.com/WordPress/wordpress-develop/blob/3bdf8b7b020be9b3b2308a4fdf6a27d5dde15dc4/src/wp-includes/comment.php#L1290

We'd have to change the name, change the docs, and keep a deprecated version of the hook for some time.

  1. We should check if other plugins are calling the function directly and assuming so, create a deprecated version of the whole function.

https://wpdirectory.net/search/01EADC773CDTAXVXRYYGB37NBT

In comment.php, we would also then have to replace the use of blacklist in the comment here: https://github.com/WordPress/wordpress-develop/blob/3bdf8b7b020be9b3b2308a4fdf6a27d5dde15dc4/src/wp-includes/comment.php#L17

and when it's used here: https://github.com/WordPress/wordpress-develop/blob/3bdf8b7b020be9b3b2308a4fdf6a27d5dde15dc4/src/wp-includes/comment.php#L818

  1. There is one unit test that uses the old function name. https://github.com/WordPress/wordpress-develop/blob/6b366c6620fdd5960cedcdf80955966a715efa82/tests/phpunit/tests/comment/wpBlacklistCheck.php#L18

After we make all other changes, we should be able to rewrite this test to use the new terms.

  1. Using blacklist_keys in the HTML attributes (name, id) of the Comment Blocklist field on the Discussion Options page.

https://github.com/WordPress/wordpress-develop/blob/8b9823f53621a63d0a47626920f6930f35db160c/src/wp-admin/options-discussion.php#L209

Change all blacklist_keys to blocklist_keys.

  1. When we change the name of the textarea, we will also have to change the name of the parameter we look for in the server code. That would be here https://github.com/WordPress/wordpress-develop/blob/50ece6d31c33b056b91fc1f204c618c76368925b/src/wp-admin/options.php#L106, and it seems we can just change the name there.
  1. This option is also set up as a default here https://github.com/WordPress/wordpress-develop/blob/8b9823f53621a63d0a47626920f6930f35db160c/src/wp-admin/includes/schema.php#L450. Change the name there.
  1. The formatting rules for this option are here https://github.com/WordPress/wordpress-develop/blob/b58973554da40b4965458d993a4703ec81e7ad28/src/wp-includes/formatting.php#L4881. Change the name there.
  1. On existing sites, the option is stored in the DB as blacklist_keys.

We would want to run a SQL query like this on update.

UPDATE $wpdb->options SET option_name = 'blocklist_keys' WHERE option_name = 'blacklist_keys';

Were other plugins accessing the option from the DB directly?

---

F) Comment in theme.php https://github.com/WordPress/wordpress-develop/blob/3bdf8b7b020be9b3b2308a4fdf6a27d5dde15dc4/src/wp-includes/theme.php#L2828

We can change Blacklist to Blocklist or reward the comment in general to avoid it.

---

G) This unit test for attachments uses the terms blacklist and whitelist in its tests of the mime type filter for uploads.

https://github.com/WordPress/wordpress-develop/blob/cfc3b57488458801c64c750cb500c98c1ef635ad/tests/phpunit/tests/post/attachments.php#L496

We can change those terms without breaking anything. The mime type filter itself (https://developer.wordpress.org/reference/hooks/upload_mimes/) doesn't use block/black/white/safe language anyway. So we can probably rename things to make the test even clearer.

---

H) This unit test for includes/schema.php has the word blacklist in a comment.

https://github.com/WordPress/wordpress-develop/blob/e72fff9ceffb41d22c48febfc5f97c0cd46f5884/tests/phpunit/tests/admin/includesSchema.php#L162

We can change that to blocklist or someone who understands what is being tested can probably rewrite the comment to be more clear.

---

I) The twentyfourteen theme has the word blacklist in a comment.

https://github.com/WordPress/wordpress-develop/blob/8b9823f53621a63d0a47626920f6930f35db160c/src/wp-content/themes/twentyfourteen/inc/featured-content.php#L236

We could say "We need to respect post ids already being excluded." or "We need to respect post ids already in the array."

---

J) The wp-admin/options.php file has a variable $whitelist_options that is used to make sure only specific options get updated on the options pages.

https://github.com/WordPress/wordpress-develop/blob/50ece6d31c33b056b91fc1f204c618c76368925b/src/wp-admin/options.php

  1. Can change all uses of that variable within this file to $safelist_options.
  1. There is a filter whitelist_options as well here https://github.com/WordPress/wordpress-develop/blob/50ece6d31c33b056b91fc1f204c618c76368925b/src/wp-admin/options.php#L209

We can change that to safelist_options. We would need to add a deprecated filter for the old use.

We would need to update the documentation.

  1. Update the error message here https://github.com/WordPress/wordpress-develop/blob/50ece6d31c33b056b91fc1f204c618c76368925b/src/wp-admin/options.php#L224

---

Here are some instances where whitelist shows up in the code. Many of these are related to the above instances of blacklist, many are not. I haven't code through all of these yet.

https://github.com/WordPress/wordpress-develop/search?q=whitelist&unscoped_q=whitelist

K) In wp-admin/includes/plugin.php there are a couple of related functions that have the word whitelist in them. add_option_whitelist() and remove_option_whitelist().

https://github.com/WordPress/wordpress-develop/blob/3bdf8b7b020be9b3b2308a4fdf6a27d5dde15dc4/src/wp-admin/includes/plugin.php#L2139

Also option_update_filter() is a callback for the whitelist_options filter and calls those functions.

There are 2 global variables $whitelist_options and $new_whitelist_options.

The $new_whitelist_options global seems to only be used by the register_setting() function https://github.com/WordPress/wordpress-develop/blob/8b9823f53621a63d0a47626920f6930f35db160c/src/wp-includes/option.php#L2117

But it's also showing up in 108 or so plugin: https://wpdirectory.net/search/01EB1MCARJHZ77593CWHEXWH3C

We could potentially update option_update_filter() function and remove the need for the $new_whitelist_options global by just using the $wp_registered_settings one and building a one time array in the right format for the add_option_whitelist function. The "group" is in the args for the options in $wp_registered_settings. We would create arrays out of those groups.

Attachments (3)

50413.diff (1.0 KB) - added by Hareesh Pillai 13 days ago.
Comment updates
50413.2.diff (1.3 KB) - added by desrosj 13 days ago.
50413.3.diff (3.5 KB) - added by desrosj 12 days ago.

Download all attachments as: .zip

Change History (32)

#1 @desrosj
3 weeks ago

  • Milestone changed from Awaiting Review to 5.5
  • Version trunk deleted

Thanks for all of this research! It was fun to tag along. Adding to 5.5, and going to explore further.

#2 @SergeyBiryukov
3 weeks ago

  • Description modified (diff)

#3 @desrosj
2 weeks ago

I've opened a PR upstream for the sodium_compat library: https://github.com/paragonie/sodium_compat/pull/121.

#4 @jorbin
2 weeks ago

Upstream PR for sodium compat has been merged

#5 @desrosj
2 weeks ago

  • Owner set to desrosj
  • Status changed from new to assigned

#6 @desrosj
2 weeks ago

Upstream PR for the Requests library: https://github.com/rmccue/Requests/pull/387

This ticket was mentioned in PR #341 on WordPress/wordpress-develop by desrosj.


2 weeks ago

  • Keywords has-patch has-unit-tests added

A working PR to replace the terms "whitelist" and "blacklist" in the WordPress codebase.

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

Todo

  • [ ] Automatically change blacklist_keys to blocklist_keys when calling get_option( 'blacklist_keys' ), just in case plugins are accessing that value directly.
  • [ ] Finish replacing occurrences of "whitelist".
  • [ ] Decide how to handle the $new_whitelist_options global variable ([some plugins](https://wpdirectory.net/search/01EB1MCARJHZ77593CWHEXWH3C) access it directly).
  • [ ] Probably a few more steps.

#8 @desrosj
2 weeks ago

  • Keywords has-unit-tests removed

I have a working branch attached. All instances of blacklist have been replaced, and about 1/3 of the instances of whitelist. I noted some questions/todos I had as I went in the PR description.

#9 @prbot
2 weeks ago

desrosj commented on PR #341:

I am going to open a new ticket for $new_whitelist_options. I am also not going to automatically switch option keys. My plan is to post a dev note after this gets committed, and I will work on submitting PRs to plugins using the old options directly/work with the plugin team to send emails to plugin authors.

#10 @desrosj
2 weeks ago

The PR is ready for review! I also branched off #50434 to tackle the $new_whitelist_options global variable separately.

#11 @prbot
2 weeks ago

Ayesh commented on PR #341:

As a person of color and someone who speaks English as a secondary language, this is looking great, in both technical and semantic ends. Thank you!

In some cases, the use of wording, for example allowed list of words makes it much more clear than just using the word whitelist.

Decide how to handle the $new_whitelist_options global variable (some plugins access it directly).

I suppose the least disruptive approach would to $new_whitelist_options = &$new_allowed_options, and submit tickets in relevant plugins to update the variable.

Another way would be to convert $new_allowed_options to an ArrayAccess implementation, and emit a deprecation notice every time an array key is accessed.

#12 @prbot
13 days ago

desrosj commented on PR #341:

I suppose the least disruptive approach would to $new_whitelist_options = &$new_allowed_options, and submit tickets in relevant plugins to update the variable.

Another way would be to convert $new_allowed_options to an ArrayAccess implementation, and emit a deprecation notice every time an array key is accessed.

@Ayesh Those are good options to explore. I have opened a different ticket to explore that as I think that needs to receive more testing. Do you mind sharing those ideas on that new ticket? [Core-#50434](https://core.trac.wordpress.org/ticket/50434)

#13 @desrosj
13 days ago

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

In 48121:

General: Remove “whitelist” and “blacklist” in favor of more clear and inclusive language.

“The WordPress open source community cares about diversity. We strive to maintain a welcoming environment where everyone can feel included.”

With this commit, all occurrences of “whitelist” and “blacklist” (with the single exception of the $new_whitelist_options global variable) are removed. A new ticket has been opened to explore renaming the $new_whitelist_options variable (#50434).

Changing to more specific names or rewording sentences containing these terms not only makes the code more inclusive, but also helps provide clarity. These terms are often ambiguous. What is being blocked or allowed is not always immediately clear. This can make it more difficult for non-native English speakers to read through the codebase.

Words matter. If one contributor feels more welcome because these terms are removed, this was worth the effort.

Props strangerstudios, jorbin, desrosj, joemcgill, timothyblynjacobs, ocean90, ayeshrajans, davidbaumwald, earnjam.
See #48900, #50434.
Fixes #50413.

#15 @desrosj
13 days ago

In 48122:

General: Reference the correct database version for 5.5.0 upgrades.

Follow up to [48121].

See #50413.

#16 @desrosj
13 days ago

  • Keywords needs-dev-note added
  • Summary changed from Enhancement: Update code and comments to use more inclusive language. to Update code and comments to remove "blacklist" and "whitelist"

Marking for a dev note. There also will need to be some follow up communication to plugin developers.

@Hareesh Pillai
13 days ago

Comment updates

#17 @Hareesh Pillai
13 days ago

50413.diff updates the comments to refer to the right function name, and a minor grammatical fix.

@desrosj
13 days ago

#18 @desrosj
13 days ago

In 48124:

Docs: Small inline documentation corrections following [48121].

Also, remove the version argument from the wp.deprecated() call in password-strength-meter.js. This argument is for specifying the version a feature will be removed, not when the feature was removed.

Props hareesh-pillai.
Fixes #50413.

#19 follow-up: @desrosj
13 days ago

Thanks @hareesh-pillai! Good catches.

I also noticed that the wp.deprecated package indicates that the provided version will remove the function, not as when it was deprecated. That will also need to be fixed, and is in 50413.2.diff.

I opened a ticket upstream requesting that the ability to specify a deprecated version is added, but the version argument should be removed for now.

#20 in reply to: ↑ 19 ; follow-ups: @ocean90
12 days ago

Replying to desrosj:

I also noticed that the wp.deprecated package indicates that the provided version will remove the function, not as when it was deprecated. That will also need to be fixed, and is in 50413.2.diff.

Instead of using wp.deprecated I think a simple window.console.log would be sufficient here. This will avoid three more dependencies.

add_option_allowed_list() and remove_option_allowed_list() sound a bit odd now. Thoughts on using add_allowed_options() and remove_allowed_options() instead?

#21 in reply to: ↑ 20 @SergeyBiryukov
12 days ago

Replying to ocean90:

add_option_allowed_list() and remove_option_allowed_list() sound a bit odd now. Thoughts on using add_allowed_options() and remove_allowed_options() instead?

(add|remove)_allowed_options() seems better to me too, would be consistent with the $allowed_options global.

#22 @SergeyBiryukov
12 days ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@desrosj
12 days ago

#23 in reply to: ↑ 20 @desrosj
12 days ago

Replying to ocean90:

I also noticed that the wp.deprecated package indicates that the provided version will remove the function, not as when it was deprecated. That will also need to be fixed, and is in 50413.2.diff.

Instead of using wp.deprecated I think a simple window.console.log would be sufficient here. This will avoid three more dependencies.

add_option_allowed_list() and remove_option_allowed_list() sound a bit odd now. Thoughts on using add_allowed_options() and remove_allowed_options() instead?

Agreed! 50413.3.diff makes these adjustments.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


12 days ago

#25 @desrosj
12 days ago

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

In 48142:

General: Rename (add|remove)_option_allowed_list() to (add|remove)_allowed_option().

The new names make the purpose more clear. It also adds consistency with the $allowed_options global variable.

Also in this change, the wp-deprecated dependency has been removed from the password strength meter in favor of a window.console.log() call to avoid adding 3 dependencies for one deprecated notice.

Props SergeyBiryukov, ocean90, desrosj
Fixes #50413.

#26 follow-up: @kebbet
10 days ago

Just noted the whitelist-term in the documentation. I think it should be updated aswell. Should a ticked be created on the meta trac for this?
https://developer.wordpress.org/reference/functions/check_upload_mimes/

#27 in reply to: ↑ 26 ; follow-up: @SergeyBiryukov
10 days ago

Replying to kebbet:

Just noted the whitelist-term in the documentation. I think it should be updated aswell. Should a ticked be created on the meta trac for this?

No need for a ticket, it will be automatically updated after WordPress 5.5 is released.

#28 in reply to: ↑ 27 @kebbet
10 days ago

Replying to SergeyBiryukov:

No need for a ticket, it will be automatically updated after WordPress 5.5 is released.

Great! Thanks for clearifying!

#29 @desrosj
10 days ago

Your comment reminded me that I needed to update the Spelling Best Practices page in the handbook. That is now updated and contains suggestions for alternative language: https://make.wordpress.org/core/handbook/best-practices/spelling/

Note: See TracTickets for help on using tickets.