Convert all calls to check_plain() in core to Drupal\Component\Utility\String::checkPlain() in core/modules/M-Z (except system/simpletest)

Assigning it.

1st patch from M to Z

Removed tag

Corrected patch.

Remove check_plain() in core to Drupal\Component\Utility\String::checkPlain().

Most of references to check_plain() replaced. However there are still a couple issues

• menu_test.module // menu_test_menu() has callback to check_plain in one menuitem, which I do not know what to do with
• views declares also String class (twice) and I am not sure if that class is the one to use inside views or not [currently String::checkPlain]

.

Views check_plain: I think right way is to go with Drupal\Component\Utility\String with an alias since basically that is the one to use for replacing check_plain() (instead of String class declared by Views).

Menu-item: I could not find any other callbacks which would use array('\Drupal\Component\Utility\String', 'checkPlain') -kind of syntax, actually there is not one anywhere else (which I could find).

Let's try again.

Let's try again.

Sorry for the double post. Files are the same.

Forgot to give an alias to String (collision with views's String), now fixed.

Re-rolling with aliased UtilityString class, fixed typo.

Re-rolling #27 and fixes from #29

run tests

Hopefully got them all now, see #22. Aliasing Drupal\Component\Utility\String might not be the correct solution.

rerolling #34

Chasing head and fixing a missing use statement in user.module.

... and the interdiff.

New attempt.

This one *will* fail however on the Menu tests. However, I've somewhat isolated it's cause: 'description callback' in menu_test does not support the array syntax per #18 and #19. See: core/includes/menu.inc => _menu_item_localize().

The same also goes for title_callback.
1/ No support for the syntax above
2/ Around line 673 this happens:

    if ($title_callback == 'check_plain') {
      $item['localized_options']['html'] = TRUE;
    }

I'd suggest opening up a separate issue which fixes that before we can get back here.

Would it not be enought to fix this and other alike callback test? After all it is just a test to see if stuff is already plain-texted.

if ($title_callback == array('\Drupal\Component\Utility\String', 'checkPlain)')) {
  $item['localized_options']['html'] = TRUE;
}

I don't think so.

1. re: title_callback == array('\Drupal\Component\Utility\String', 'checkPlain)')

$title_callback can either be a string (function name) or an array (method call). Since the type of $title_callback is ambiguous, I don't think a condition like this could work.

2. We should think further then just menu_test. It's not because this is the first and only occurrence of String::checkPlain in core, it's not possible that someone will try to call other core class methods or contrib class methods as a *_callback.

I think this merits further discussion in a separate issue.

See: #2101803: Allow class methods to be set as menu *_callback

Hm.

I think this issue is blocked because description_callback in menu_test.module is not supporting the new static checkPlain() method.

#2076085: Resolve the need for a 'title callback' using the route pretty much converts title_callback to the new yml _title_callback property. There is no similar conversion for description_callback at this point, though.

However, since hook_menu is being phased out for the new routing system, I wonder if we can work around this by either:

1. Ignoring check_plain() functionality in to-be removed hooks like hook_menu alltogether
2. Change the test to use the new _title_callback functionality instead, but that needs to be checked of with the routing/menu maintainers.
3. Wait until someone comes up with a decent _description_callback compatibility within the routing system.

new try

48: check_plain_to_String_checkPlain-2089471-48.patch queued for re-testing.

new try.

changed status for patch testing.

Update issue.

A half of those patches are for views modules.
Seems that this issue is too big to be done at once.

I have created separated task for views/views_ui modules:
#2187829: Convert all calls to check_plain() in core to Drupal\Component\Utility\String::checkPlain() in core/modules/views

Re-rolling now

This is a reroll after the views issue got in, plus it fixes 3 uses in array_map.

There were lots that still needed replacing in the system module, but simpletest had some that could prove tricky (simpletest.install scares me) and we've already got a 66k patch so I'll split those into a separate issue.

I've removed references to views from the issue title/summary, as if the odd one slips back in this is now the right issue to fix it in.

The array_map uses needed to be fully qualified.

1. +++ b/core/modules/text/lib/Drupal/text/Tests/Formatter/TextPlainUnitTest.php
   @@ -145,7 +146,7 @@ protected function renderEntityFields(ContentEntityInterface $entity, EntityView
   +   * @see String::checkPlain()
   

   Classes should be fully qualified (i.e. start \Drupal) in @see blocks.

2. +++ b/core/modules/user/lib/Drupal/user/Plugin/views/field/Name.php
   @@ -84,7 +85,7 @@ protected function renderLink($data, ResultRow $values) {
            // This is an anonymous user, and we're overriting the text.
   

   Are we allowed to fix this spelling mistake as we're making a change next to it (overriting > overwriting)?

3. +++ b/core/modules/user/user.module
   @@ -564,7 +565,7 @@ function user_preprocess_block(&$variables) {
   - *   this result must ensure that check_plain() is called on it before it is
   + *   this result must ensure that String::checkPlain() is called on it before it is
   

   Over 80 characters

I've checked the rest of the patch, including making sure no function parameters have been changed and broken by a reroll, and it all looks good. I think we can RTBC once these nitpicks are fixed.

Fixed all items in #66. I agree we should fix the spelling error now while it's been noted.

That fixes my concerns, assuming the tests pass this is RTBC.

2089471-remove-check_plain-67.patch no longer applies.

error: patch failed: core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php:2995
error: core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php: patch does not apply

Reroll. Just a context change in WebTestBase, theme > _theme, so back to RTBC

2089471-remove-check_plain-70.patch no longer applies.

error: patch failed: core/modules/user/user.module:564
error: core/modules/user/user.module: patch does not apply

Just context changes, looks good.

2089471-remove-check_plain-72.patch no longer applies.

error: patch failed: core/modules/user/user.module:1
error: core/modules/user/user.module: patch does not apply

This was fixed on the meta