Part of #2089331: [meta] Replace calls to check_plain() with Drupal\Component\Utility\String::checkPlain().

CommentFileSizeAuthor
#28 2089461-core-lib-check_plain.patch2.83 KBlongwave
#21 drupal-2089461-21-switch-check_plain-to-String-checkPlain.patch7.23 KBdeneo
#20 drupal-2089461-20-switch-check_plain-to-String-checkPlain.patch7.23 KBdeneo
#17 drupal-2089461-17-switch-check_plain-to-String-checkPlain.patch8.84 KBdsdeiz
#14 drupal-2089461-14-switch-check_plain-to-String-checkPlain.patch8.88 KBAlan D.
#12 drupal-2089461-12-switch-check_plain-to-String-checkPlain.patch9 KBAlan D.
#9 2089461-9.patch18.24 KBstpaultim
#6 2089461-6.patch8.91 KBthedavidmeister
#5 2089461-5.patch8.91 KBthedavidmeister
#5 2089461-2-5-interdiff.txt518 bytesthedavidmeister
#2 2089461-2.patch8.63 KBthedavidmeister
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

thedavidmeister’s picture

thedavidmeister CreditAttribution: thedavidmeister commented
Assigned: Unassigned » thedavidmeister
thedavidmeister’s picture

thedavidmeister CreditAttribution: thedavidmeister commented
Status: Active » Needs review
FileSize
2089461-2.patch8.63 KB
thedavidmeister’s picture

thedavidmeister CreditAttribution: thedavidmeister commented
Assigned: thedavidmeister » Unassigned

Status: Needs review » Needs work

The last submitted patch, 2089461-2.patch, failed testing.

thedavidmeister’s picture

thedavidmeister CreditAttribution: thedavidmeister commented
Status: Needs work » Needs review
FileSize
2089461-2-5-interdiff.txt518 bytes
2089461-5.patch8.91 KB
thedavidmeister’s picture

thedavidmeister CreditAttribution: thedavidmeister commented
FileSize
2089461-6.patch8.91 KB

needed reroll

rpsu’s picture

rpsu
he/him
Primary language Finnish
CreditAttribution: rpsu commented
Status: Needs review » Reviewed & tested by the community

No check_plain() left after this patch, so RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Patch no longer applies.

stpaultim’s picture

stpaultim CreditAttribution: stpaultim commented
Status: Needs work » Needs review
Issue tags: -Novice, -Needs reroll
FileSize
2089461-9.patch18.24 KB

Reroll

stpaultim’s picture

stpaultim CreditAttribution: stpaultim commented
Issue tags: +Novice

Didn't mean to remove "Novice" tag.

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Status: Needs review » Needs work

You accidentally added the file 2089461-6.patch

I find it best to always do something like this to avoid creating patches with the checked out directory :)

git diff > ../patches/patch-file.patch

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Status: Needs work » Needs review
FileSize
drupal-2089461-12-switch-check_plain-to-String-checkPlain.patch9 KB

Re-roll

Status: Needs review » Needs work

The last submitted patch, drupal-2089461-12-switch-check_plain-to-String-checkPlain.patch, failed testing.

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
FileSize
drupal-2089461-14-switch-check_plain-to-String-checkPlain.patch8.88 KB

Even a simple re-roll should be tested. Me bad. \Drupal\Component\Utility\String was already introduced in #2043757: Remove drupal_(string) methods out of diffengine.

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Status: Needs work » Needs review
areke’s picture

areke CreditAttribution: areke commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs reroll

The patch doesn't apply anymore. It needs to be rerolled again.

dsdeiz’s picture

dsdeiz CreditAttribution: dsdeiz commented
FileSize
drupal-2089461-17-switch-check_plain-to-String-checkPlain.patch8.84 KB

Re-rolling.

dsdeiz’s picture

dsdeiz CreditAttribution: dsdeiz commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 17: drupal-2089461-17-switch-check_plain-to-String-checkPlain.patch, failed testing.

deneo’s picture

deneo CreditAttribution: deneo commented
FileSize
drupal-2089461-20-switch-check_plain-to-String-checkPlain.patch7.23 KB
deneo’s picture

deneo CreditAttribution: deneo commented
FileSize
drupal-2089461-21-switch-check_plain-to-String-checkPlain.patch7.23 KB
areke’s picture

areke CreditAttribution: areke commented
Status: Needs work » Needs review

The patch applies and changes the check_plain() calls correctly, but let's see if the test bot is happy with the changes.

areke’s picture

areke CreditAttribution: areke commented
Status: Needs review » Reviewed & tested by the community

Ok, this looks good. Thank you!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

thedavidmeister’s picture

thedavidmeister CreditAttribution: thedavidmeister commented
Status: Closed (fixed) » Active
Issue tags: -Needs reroll

There are some more calls to check_plain() in:

- core/lib/Drupal/Core/Field/WidgetBase.php

There are references to check_plain() in the docs in:

- core/lib/Drupal/Core/Utility/Token.php

thedavidmeister’s picture

thedavidmeister CreditAttribution: thedavidmeister commented
Status: Active » Needs work
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Needs work » Needs review
FileSize
2089461-core-lib-check_plain.patch2.83 KB
thedavidmeister’s picture

thedavidmeister CreditAttribution: thedavidmeister commented
Status: Needs review » Reviewed & tested by the community

I'm going to go ahead and RTBC that unless the testbots disagree.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 28: 2089461-core-lib-check_plain.patch, failed testing.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Needs work » Needs review

Previous test result:

Fatal error: Call to a member function isPermanent() on a non-object in /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/image/lib/Drupal/image/Tests/ImageFieldDisplayTest.php on line 272
FATAL Drupal\image\Tests\ImageFieldDisplayTest: test runner returned a non-zero error code (255).

Suspect this is a disk error on the testbot, let's see.

28: 2089461-core-lib-check_plain.patch queued for re-testing.

thedavidmeister’s picture

thedavidmeister CreditAttribution: thedavidmeister commented
Status: Needs review » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.