Skip to content

Comments

[4.0] whitelist strings#32030

Merged
wilsonge merged 8 commits intojoomla:4.0-devfrom
brianteeman:allowed
Feb 14, 2021
Merged

[4.0] whitelist strings#32030
wilsonge merged 8 commits intojoomla:4.0-devfrom
brianteeman:allowed

Conversation

@brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Jan 14, 2021

Updates the strings to use Allowed and Forbidden instead of whitelist and blacklist. This still leaves the use of whitelist in com_csp as I can't actually see a list at all. @zero-24 please clarify

Uses the terms allowed and forbidden instead of whitelist and blacklist

Continuation of #29703

Previous comment

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

Updates the strings to use Allowed and Forbidden instead of whitelist and blacklist. This still leaves the use of whitelist in com_csp as I can't actually see a list at all. @zero-24 please clarify
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Jan 14, 2021
@zero-24
Copy link
Contributor

zero-24 commented Jan 14, 2021

The usage of whitelist in com_csp can be changed to allowlist as well without any problems.

@brianteeman
Copy link
Contributor Author

But where is this list?

@zero-24
Copy link
Contributor

zero-24 commented Jan 14, 2021

The collected and approved urls are put together as 'allowlist' for example or in this case Allowed domains to "use" whatever directive we are talking about.

@wilsonge
Copy link
Contributor

For the code. It's going to be a bit situational. For things like https://github.com/joomla/joomla-cms/pull/29703/files#diff-108363cbc354ffab84b1fd1e834d0406f74132c0dd946ec7f8445e3c4ad67e1bR136 which was soley a variable used in a function (i.e. not accessible to anyone) then it's fine to change obviously.

Some of the other changes like https://github.com/joomla/joomla-cms/pull/29703/files#diff-e6d69a506c2984b154921451397a94d38a0513f0f40da9a531cc311a40daf624R145 which changed the term saved to db - less sure. As we're still going to always read both formats out not really sure it's a major issue (and personally it's one of those things where I wouldn't even deprecate for bs5 - just read them both forever - because assets are rarely going to change and I think it's asking too much)

I think the major thing in the old PR I wouldn't maybe feel comfortable changing anymore would be the ListModel stuff - because that's very widely extended by 3rd parties.

@bembelimen
Copy link
Contributor

bembelimen commented Jan 15, 2021

Thank you for your PR! In my opinion we should do all or should not do it at all. Otherwise it is just a "sham". That's what I tried at least with the PR. I think I found all the appearance and made it B/C, so you could just look up the lines and fix them. My problem was just to be not native english...

@brianteeman
Copy link
Contributor Author

@bembelimen I will take a look this weekend. The key issue is that its not a simple search replace so each use has to be checked.

@adj9
Copy link

adj9 commented Jan 16, 2021

I have not tested this item.

But where are the terms to be verified?


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

@brianteeman
Copy link
Contributor Author

@bembelimen I think I have got all the code changes you suggested plus one additional one. Please can you check.

@bembelimen
Copy link
Contributor

Didn't found anything, not sure if this b/c thing in the UsersModel is needed, because it's merged anyways, but otherways great job, thanks 👍

@bembelimen
Copy link
Contributor

@brianteeman
Copy link
Contributor Author

@bembelimen not sure as I'm not sure about the deprecation at such a late time. Will leave that decision to @wilsonge

@ceford
Copy link
Contributor

ceford commented Jan 25, 2021

I have tested this item ✅ successfully on 25686ef

I code searched for BLACKLIST, BLACK_LIST, blacklist and Blacklist (and white versions) before and after applying PR. The only ones unaffected were in libraries/vendor and media/vendor. Except for Blacklist in libraries/src/MVC/Model/ListModel.php - was that covered in the discussion?


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

@wilsonge
Copy link
Contributor

For the component specific implementation I think it's fine to remove the old properties. In list model you're right to keep B/C

@ghost
Copy link

ghost commented Jan 31, 2021

I have tested this item ✅ successfully on 25686ef


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

@brianteeman
Copy link
Contributor Author

@wilsonge I'm not sure I understand what you want to do here.

@brianteeman
Copy link
Contributor Author

I guess its not that important after all

@wilsonge wilsonge merged commit 421c3a1 into joomla:4.0-dev Feb 14, 2021
@wilsonge
Copy link
Contributor

Thanks!

@wilsonge wilsonge added this to the Joomla 4.0 milestone Feb 14, 2021
@brianteeman
Copy link
Contributor Author

Now I see your commit I understand your comment. I didnt before - sorry

@brianteeman brianteeman deleted the allowed branch February 14, 2021 00:30
@wilsonge
Copy link
Contributor

Don't worry about it. Sorry for not replying promptly!

brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request Jul 14, 2025
Removes code deprecated in favour of forbiddenlist

Original PR with explanation of the deprecation joomla#32030
brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request Jul 14, 2025
Changes the comment whitelist to allowed

Original PR and discussion joomla#32030

Core review only

Signed-off-by: BrianTeeman <brian@teeman.net>
@brianteeman brianteeman mentioned this pull request Jul 14, 2025
softforge pushed a commit that referenced this pull request Jul 23, 2025
Removes code deprecated in favour of forbiddenlist

Original PR with explanation of the deprecation #32030
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Language Change This is for Translators

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants