Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Lets remove usage of "blacklist" and "whitelist" in Filter module.
They are:
- An historic bad labelling of people
- Provide no context: "what is listed in them"?
Proposed resolution
core/modules/filter/src/Element/TextFormat: s/$blacklist/$keys_not_to_copy/
Otherwise, it's all comments. Generally replace "whitelist" with "allowed" and "blacklist" with "denied".
Remaining tasks
Come up with replacement names.Decide what needs BC + deprecation and what can be changed directly.Everything are comments and local variables. No BC, deprecations or CR needed.Fix everything:core/modules/filter/tests/src/Kernel/FilterKernelTest.phpcore/modules/filter/src/Entity/FilterFormat.phpcore/modules/filter/src/Plugin/Filter/FilterHtml.phpcore/modules/filter/src/Element/TextFormat.phpcore/modules/filter/src/FilterFormatInterface.php
Reviews / improvements.- RTBC.
- Commit.
User interface changes
None
API changes
N/A
Data model changes
N/A
Release notes snippet
N/A
Comment | File | Size | Author |
---|---|---|---|
#20 | 3151101-20.patch | 12.17 KB | dww |
#10 | interdiff_6-10.txt | 10.1 KB | Deepak Goyal |
#10 | 3151101-10.patch | 11.95 KB | Deepak Goyal |
#6 | d9whiteblacklist-3151101-6.patch | 11.37 KB | shetpooja04 |
Comments
Comment #2
ashishdalviI will be picking this issue today.
Comment #3
dwwFlesh out Remaining tasks.
Comment #4
ashishdalviComment #5
shetpooja04 CreditAttribution: shetpooja04 at QED42 commentedComment #6
shetpooja04 CreditAttribution: shetpooja04 at QED42 commentedI have worked on issue as per the issue description
Comment #8
alexpottLet's call this $keys_not_to_copy as $blocklist or the original term is not actually that helpful here.
We can remove the terms between the brackets. They're not necessary.
allowed
allowed
and there are no forbidden tags, the effectively nothing is allowed.
Let's go with:
allowed
I think we can remove the "according to a list". There's a parameter called $allowed_attributes - I think that's enough.
The list of allowed attributes as an array...
the list of allowed attributes.
the list of allowed tags more specific.
allowing
The list of allowed values for the 'dir' attribute...
allowed.
This is a strange @todo - it would be nice to rewrite this with using the allowlist term.
allowed tags
allow
allowed
Comment #9
Deepak Goyal CreditAttribution: Deepak Goyal at Srijan | A Material+ Company for Drupal India Association commentedComment #10
Deepak Goyal CreditAttribution: Deepak Goyal at Srijan | A Material+ Company for Drupal India Association commentedHi @alexpott
Made changes as you suggested please review.
And #8.16 is pending I am not getting this.
Comment #11
Deepak Goyal CreditAttribution: Deepak Goyal at Srijan | A Material+ Company for Drupal India Association commentedComment #12
alexpottThe comment needs re-flowing now.
it's
will fit on the previous line now.There's a double space in front of and.
Remove attributes not in the list of allowed attributes.
Check the allowed attribute value list.
Can re-flow comment. I think "case" now fits on the preceding line.
As we changing this line we can remove the double space after the fullstop.
I think
might work here.
Comment #13
Deepak Goyal CreditAttribution: Deepak Goyal at Srijan | A Material+ Company for Drupal India Association commentedComment #14
Deepak Goyal CreditAttribution: Deepak Goyal at Srijan | A Material+ Company for Drupal India Association commentedHi @alexpott
Made changes as you suggested please review.
For #12.5 "case" is not fits on the preceding line.
Comment #15
dwwAlmost there, thanks! Just a few lingering things, then this is RTBC:
A) 80 char flow: "and" can move up to the previous line.
B) s/the/then/ effectively...
'Filters' needs an 's'. Also, I think it's still worth mentioning the allowed list. Something like:
"Filters attributes on an element according to an allow list." (or something).
But otherwise, we're done. Before:
After:
(and for good measure):
Comment #16
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedHere I have addressed #15.1 and #15.2 points of comment #15.
Comment #17
dwwExcellent, thanks!
Comment #18
quietone CreditAttribution: quietone as a volunteer commentedI was curious to know what words whitelist and blacklist were being replaced with and found that the IS states 'TBD'. So I looked at the patch for answer and that raised questions.
Surely 'allow' is a verb and not an adjective. Instead of 'allow list' maybe 'listed of allowed values'?
Shouldn't this be 'Check the list of allowed of attribute values'?
For the IS the proposed resolution needs to be updated with the suggested replacements for whitelist/blacklist. The remaining tasks section shows that there are 5 tasks still to do. Is that one tasks queries the need for BC but there is nothing in the issue that addresses that point.
Sorry, back to NW.
Comment #19
dwwThanks, @quietone! Right you are. I even bothered to make a good list of remaining tasks, then didn't check we got them all. Updated.
core/modules/filter/filter.module was a false positive, so I removed it. Everything else is indeed gone (per greps at the end of #15).
Re: specific review points in #18:
edof allowed values"ofattribute values."otherwise, +1 to those improvements.
Thanks again,
-Derek
Comment #20
dwwNow it just needs RTBC. ;)
Comment #21
quietone CreditAttribution: quietone as a volunteer commented@derek, thanks for the changes.
I reviewed the interdiff and the suggested changes were agreed to and made. The IS has been updated. Yeah, I don't have to read the whole patch to learn the solution!
Back to RTBC
Comment #22
alexpottCommitted 1d44c8e and pushed to 9.1.x. Thanks!
Will backport once the release is out and the freeze is over.
Comment #24
alexpottOops meant to leave this at RTBC
Comment #25
alexpottCommitted and pushed aca70f2cd2 to 9.0.x and 7fafcd6952 to 8.9.x. Thanks!
Backported to 8.9.x as this is docs and internal variable names only.
Comment #28
alexpott