Skip to content

Comments

Remove whitelist/blacklist terms#29703

Closed
bembelimen wants to merge 5 commits intojoomla:4.0-devfrom
bembelimen:4.0/white-blacklist
Closed

Remove whitelist/blacklist terms#29703
bembelimen wants to merge 5 commits intojoomla:4.0-devfrom
bembelimen:4.0/white-blacklist

Conversation

@bembelimen
Copy link
Contributor

Summary of Changes

This PR removed all "whitelist" and "blacklist" terms in Joomla! 4.
Existing variables/languages strings are deprecated when needed and a replacement is implemented. Otherwise the variables/strings are renamed.

Testing Instructions

Check the area where the files are changed and look in Joomla! if everything is still working and has the new naming

@wilsonge

@bembelimen bembelimen requested a review from HLeithner as a code owner June 18, 2020 22:18
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Jun 18, 2020
@brianteeman
Copy link
Contributor

Allowlist and disallowlist are not English.

Yes I know this is what Drupal did but it doesn't change the facts.

The opposite of allow is deny

List is not a suffix it is a new word.

On a personal note this is not the same as master/slave which has historically racist meaning. Blacklist refers to a black colour book.

@richard67
Copy link
Member

@bembelimen PHPCS errors reported by Drone: https://ci.joomla.org/joomla/joomla-cms/33260/1/9

@bembelimen
Copy link
Contributor Author

Allowlist and disallowlist are not English.

Yes I know this is what Drupal did but it doesn't change the facts.

The opposite of allow is deny

List is not a suffix it is a new word.

Thanks for your correction, as non-native speaker it's sometimes not easy to hit the difference. My thoughts were: I allow tags/attributes or I disallow them. What would you suggest as proper wording?

On a personal note this is not the same as master/slave which has historically racist meaning. Blacklist refers to a black colour book.

I can't judge for others if someone is offended or not as I'm not affected, I was asked to create a PR, so I'm open for the best solution for everyone.

@brianteeman
Copy link
Contributor

"allow list" and "deny list"

$disallowedAttributes = array_merge($disallowedAttributes, $tempAttributes);
}
elseif ($filterType === 'CBL')
// "CBL" is deprecated in Joomla! 4, will be removed in Joomla! 5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we not need a proper @ deprecated ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answering my own question - later on in the code you use
// @deprecated in 4.0 remove in Joomla 5.0

@N6REJ
Copy link
Contributor

N6REJ commented Jun 19, 2020

blacklist & whitelist are long standing industry terms and not the least bit racially related.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29703.

@wilsonge
Copy link
Contributor

wilsonge commented Jun 20, 2020

@N6REJ and the industry is increasingly moving away from these terms:

PHP is discussing removing the terminology: https://externals.io/message/110515#110515

PHPUnit has removed the terminology: sebastianbergmann/phpunit@fe9223c#diff-7c9432f8518d7bc639d398ad1f994711

Whether you agree with the racial link or not the industry is moving away from the term so it's not really a reason either.

  • I suggest for our tags using the term blocked list - two advantages - one is that it's actually describing what's happening. But secondly we can still use the abbreviation BL in our code without making any b/c breaks :)
  • For workflows I'd use "Excluded Extensions"

@particthistle
Copy link
Member

I have tested this item 🔴 unsuccessfully on 383b805

Logging a number of identified issues and examples in further comments.

Also based on the above discussion, @bembelimen is there additional changes to be made to some of the default wording values?

There's some other grammar items I've spotted looking at the files list.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29703.

COM_CONFIG_FIELD_FILTERS_CUSTOM_BLACK_LIST="Custom Blacklist"
COM_CONFIG_FIELD_FILTERS_DEFAULT_BLACK_LIST="Default Blacklist"
COM_CONFIG_FIELD_FILTERS_CUSTOM_DISALLOW_LIST="Custom DisallowList"
COM_CONFIG_FIELD_FILTERS_DEFAULT_DISALLOW_LIST="Default DisallowList"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing language value following change:

COM_CONFIG_FIELD_FILTERS_DEFAULT_ALLOW_LIST="Custom AllowList"

Fixes: Global Configuration > Text Filters > Change in default text after removal of predecessor.
image

Further change based on the discussion on AllowList vs Allow List may need additional edit.

COM_MEDIA_FIELD_IGNORED_EXTENSIONS_DESC="Ignored file extensions for MIME type checking and restricted uploads."
COM_MEDIA_FIELD_IGNORED_EXTENSIONS_LABEL="Ignored Extensions"
COM_MEDIA_FIELD_ILLEGAL_MIME_TYPES_DESC="A comma separated list of illegal MIME types to upload (blacklist)."
COM_MEDIA_FIELD_ILLEGAL_MIME_TYPES_DESC="A comma separated list of illegal MIME types to upload."
Copy link
Member

@particthistle particthistle Jun 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
COM_MEDIA_FIELD_ILLEGAL_MIME_TYPES_DESC="A comma separated list of illegal MIME types to upload."
COM_MEDIA_FIELD_ILLEGAL_MIME_TYPES_DESC="A comma separated list of MIME types that can not be uploaded."

Improves the phrasing of the field description for Illegal MIME types, as ones listed in this field can not be uploaded.
image

to:
image

JWORKFLOW_EXTENSION_BLACKLIST_LABEL="Extension Blacklist"
JWORKFLOW_EXTENSION_WHITELIST_DESCRIPTION="Activate this plugin only for listed extensions. If used all other extensions are disabled."
JWORKFLOW_EXTENSION_WHITELIST_LABEL="Extension Whitelist"
JWORKFLOW_EXTENSION_DISALLOW_LIST_DESCRIPTION="Disable this plugin for listed extensions."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To test these strings:
System > Plugins > search for Workflow plugins.

@bembelimen side question: Do the plugins typically function on Articles component when the Extension Whitelist is empty, or should there be a default set for the Workflow plugins?

COM_MEDIA_FIELD_IGNORED_EXTENSIONS_DESC="Ignored file extensions for MIME type checking and restricted uploads."
COM_MEDIA_FIELD_IGNORED_EXTENSIONS_LABEL="Ignored Extensions"
COM_MEDIA_FIELD_ILLEGAL_MIME_TYPES_DESC="A comma separated list of illegal MIME types to upload (blacklist)."
COM_MEDIA_FIELD_ILLEGAL_MIME_TYPES_DESC="A comma separated list of illegal MIME types to upload."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to administrator language file:

Suggested change
COM_MEDIA_FIELD_ILLEGAL_MIME_TYPES_DESC="A comma separated list of illegal MIME types to upload."
COM_MEDIA_FIELD_ILLEGAL_MIME_TYPES_DESC="A comma separated list of MIME types that can not be uploaded."

@particthistle
Copy link
Member

I doubt any instances have been missed in applying the blacklists, but without significantly testing actual values, it's hard to check all of that.

The few language suggestions that I'm making clarify the purposes of the fields or what the allow/disallow lists are doing.

I agree with @wilsonge on the better terminology pairings: "Allow List" and "Blocked List" as blocking vs disallowing are slightly different terms, and blocking is what the lists are doing.

Copy link
Member

@particthistle particthistle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the files again and formalised the wording to show as separate words. Quality improvement in language use.

  • DisallowList becomes Disallow List
  • AllowList becomes Allow List

COM_CONFIG_FIELD_FILESYSTEM_PATH_LABEL="Session Save Path"
COM_CONFIG_FIELD_FILTERS_CUSTOM_BLACK_LIST="Custom Blacklist"
COM_CONFIG_FIELD_FILTERS_DEFAULT_BLACK_LIST="Default Blacklist"
COM_CONFIG_FIELD_FILTERS_CUSTOM_DISALLOW_LIST="Custom DisallowList"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
COM_CONFIG_FIELD_FILTERS_CUSTOM_DISALLOW_LIST="Custom DisallowList"
COM_CONFIG_FIELD_FILTERS_CUSTOM_DISALLOW_LIST="Custom Disallow List"

Formalise the wording to show as separate words.

COM_CONFIG_FIELD_FILTERS_CUSTOM_BLACK_LIST="Custom Blacklist"
COM_CONFIG_FIELD_FILTERS_DEFAULT_BLACK_LIST="Default Blacklist"
COM_CONFIG_FIELD_FILTERS_CUSTOM_DISALLOW_LIST="Custom DisallowList"
COM_CONFIG_FIELD_FILTERS_DEFAULT_DISALLOW_LIST="Default DisallowList"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
COM_CONFIG_FIELD_FILTERS_DEFAULT_DISALLOW_LIST="Default DisallowList"
COM_CONFIG_FIELD_FILTERS_DEFAULT_DISALLOW_LIST="Default Disallow List"

Formalise the wording to show as separate words.

COM_CONFIG_FIELD_FILTERS_CUSTOM_DISALLOW_LIST="Custom DisallowList"
COM_CONFIG_FIELD_FILTERS_DEFAULT_DISALLOW_LIST="Default DisallowList"
; Deprecate in Joomla! 4, remove in Joomla! 5
COM_CONFIG_FIELD_FILTERS_CUSTOM_BLACK_LIST="Custom DisallowList"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
COM_CONFIG_FIELD_FILTERS_CUSTOM_BLACK_LIST="Custom DisallowList"
COM_CONFIG_FIELD_FILTERS_CUSTOM_BLACK_LIST="Custom Disallow List"

Formalise the wording to show as separate words.

; Deprecate in Joomla! 4, remove in Joomla! 5
COM_CONFIG_FIELD_FILTERS_CUSTOM_BLACK_LIST="Custom DisallowList"
; Deprecate in Joomla! 4, remove in Joomla! 5
COM_CONFIG_FIELD_FILTERS_DEFAULT_BLACK_LIST="Default DisallowList"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
COM_CONFIG_FIELD_FILTERS_DEFAULT_BLACK_LIST="Default DisallowList"
COM_CONFIG_FIELD_FILTERS_DEFAULT_BLACK_LIST="Default Disallow List"

Formalise the wording to show as separate words.

COM_CONFIG_FIELD_FILTERS_NO_FILTER="No Filtering"
COM_CONFIG_FIELD_FILTERS_NO_HTML="No HTML"
COM_CONFIG_FIELD_FILTERS_WHITE_LIST="Whitelist"
COM_CONFIG_FIELD_FILTERS_ALLOW_LIST="AllowList"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
COM_CONFIG_FIELD_FILTERS_ALLOW_LIST="AllowList"
COM_CONFIG_FIELD_FILTERS_ALLOW_LIST="Allow List"

Formalise the wording to show as separate words.

// Remove the whitelisted tags and attributes from the black-list.
$blackListTags = array_diff($blackListTags, $whiteListTags);
$blackListAttributes = array_diff($blackListAttributes, $whiteListAttributes);
// Remove the allowed tags and attributes from the disallowList.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Remove the allowed tags and attributes from the disallowList.
// Remove the allowed tags and attributes from the Disallow List.

Formalise the wording to show as separate words.


// Remove whitelisted tags from filter's default blacklist
if ($whiteListTags)
// Remove allowed tags from filter's default disallowList
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Remove allowed tags from filter's default disallowList
// Remove allowed tags from filter's default Disallow List

Formalise the wording to show as separate words.


// Remove whitelisted attributes from filter's default blacklist
if ($whiteListAttributes)
// Remove allowed attributes from filter's default disallowList
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Remove allowed attributes from filter's default disallowList
// Remove allowed attributes from filter's default Disallow List

Formalise the wording to show as separate words.

}
// Whitelists take third precedence.
elseif ($whiteList)
// AllowLists take third precedence.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// AllowLists take third precedence.
// Allow Lists take third precedence.

Formalise the wording to show as separate words.

}
// Blacklists take second precedence.
elseif ($blackList)
// DisallowList take second precedence.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// DisallowList take second precedence.
// Disallow List take second precedence.

Formalise the wording to show as separate words.

@particthistle
Copy link
Member

particthistle commented Jul 25, 2020

Will leave someone to make a final decision on this for the desired combination.

Currently the reviewed code has "Allow List" and "Disallow List" now throughout - matching the language tags. Also follows familiar terminology used in Robots.txt.

@wilsonge should it be "Blocked List" in which case I'm happy to go through and review my review.

@bembelimen once review has been accepted I'll then test again.

@Quy Quy added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Sep 14, 2020
@bembelimen
Copy link
Contributor Author

I think, if this happens, it should be made from scratch again to cover the new suggestion from Brian.

Probably @particthistle you want to give it a try?

@bembelimen bembelimen closed this Jan 14, 2021
@particthistle
Copy link
Member

Someone can feel free to open an RFC discussion to put forward an agreed upon linguistic standard to use, and then we can build this again.

@brianteeman
Copy link
Contributor

I just spent some time looking at this and it will never be a simple search and replace. After thinking about it for a while I think in most cases it will be enough to just changed to allowed and forbidden but there will be some occasions where for linguistic reasons it will need to be allowed list and forbidden list

The second issue is should this be everywhere or just in the visible strings. Everywhere means changing comments and variable names and to me the time to do that has now passed. But some might argue that if you still have a variable called $blacklist then you're not really desensitizing the language.

If @wilsonge and @bembelimen can make a decision on that I can easily do the work

@HLeithner
Copy link
Member

Changing the variable names it self is not the problem but the b/c break that can happen if they are public, also formfields are hard to change and have to be migrated (or both fields are considered till 5.0).

@bembelimen bembelimen deleted the 4.0/white-blacklist branch January 23, 2024 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Conflicting Files Language Change This is for Translators Updates Requested Indicates that this pull request needs an update from the author and should not be tested.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants