core/modules is so large, so splitting to several issues.
This is issue for views and views_ui modules.

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

CommentFileSizeAuthor
#23 2187829-views-check-plain-23.patch34.17 KBianthomas_uk
#19 interdiff.txt7.06 KBlongwave
#19 2187829-views-check-plain-19.patch34.18 KBlongwave
#17 2187829-views-check-plain-17.patch34.18 KBlongwave
#15 interdiff.txt5.86 KBlongwave
#15 2187829-views-check-plain-15.patch33.96 KBlongwave
#13 2187829-views-check-plain-13.patch33.29 KBrichard.c.allen2386
#7 interdiff.txt1.03 KBlongwave
#7 2187829-views-check-plain-7.patch33.09 KBlongwave
#5 2187829-views-check-plain-5.patch33.06 KBlongwave
#2 drupal-check-plain-views-2187829-2-1.patch28.5 KBIshaDakota
#2 interdiff.txt8.11 KBIshaDakota
#1 drupal-check-plain-views-2187829-1.patch19.61 KBIshaDakota
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

IshaDakota’s picture

IshaDakota CreditAttribution: IshaDakota commented
Status: Needs work » Needs review
FileSize
drupal-check-plain-views-2187829-1.patch19.61 KB

Replacing all check_plain() with String::check_plain().

IshaDakota’s picture

IshaDakota CreditAttribution: IshaDakota commented
FileSize
interdiff.txt8.11 KB
drupal-check-plain-views-2187829-2-1.patch28.5 KB

First patch left out views_ui (and a few others).

The last submitted patch, 1: drupal-check-plain-views-2187829-1.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 2: drupal-check-plain-views-2187829-2-1.patch, failed testing.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Needs work » Needs review
FileSize
2187829-views-check-plain-5.patch33.06 KB

Status: Needs review » Needs work

The last submitted patch, 5: 2187829-views-check-plain-5.patch, failed testing.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Needs work » Needs review
FileSize
2187829-views-check-plain-7.patch33.09 KB
interdiff.txt1.03 KB

Status: Needs review » Needs work

The last submitted patch, 7: 2187829-views-check-plain-7.patch, failed testing.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented

7: 2187829-views-check-plain-7.patch queued for re-testing.

The last submitted patch, 7: 2187829-views-check-plain-7.patch, failed testing.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented

7: 2187829-views-check-plain-7.patch queued for re-testing.

The last submitted patch, 7: 2187829-views-check-plain-7.patch, failed testing.

richard.c.allen2386’s picture

richard.c.allen2386 CreditAttribution: richard.c.allen2386 commented
Status: Needs work » Needs review
FileSize
2187829-views-check-plain-13.patch33.29 KB

Tried re-rolling the patch, new to the issue so no idea if this is going to fix the issue but it applies cleanly to current dev and install.php is running fine.

Status: Needs review » Needs work

The last submitted patch, 13: 2187829-views-check-plain-13.patch, failed testing.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Needs work » Needs review
FileSize
2187829-views-check-plain-15.patch33.96 KB
interdiff.txt5.86 KB

Similarly the installation worked fine for me in the UI, but I managed to reproduce the failure with drush.

The problem is a clash between the Views classes named "String" and the import of \Drupal\Component\Utility\String via use statements. See the interdiff for details.

Status: Needs review » Needs work

The last submitted patch, 15: 2187829-views-check-plain-15.patch, failed testing.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Needs work » Needs review
FileSize
2187829-views-check-plain-17.patch34.18 KB
ianthomas_uk’s picture

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

Good catch, but it should be UtilityString according to the coding standards (https://drupal.org/node/1353118 class aliasing section). Other than that the patch looks good.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Needs work » Needs review
FileSize
2187829-views-check-plain-19.patch34.18 KB
interdiff.txt7.06 KB

StringUtility is much more readable and makes more sense to me, but coding standards are coding standards.

Status: Needs review » Needs work

The last submitted patch, 19: 2187829-views-check-plain-19.patch, failed testing.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented

19: 2187829-views-check-plain-19.patch queued for re-testing.

longwave’s picture

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

ianthomas_uk
he/him
Primary language English
Location Brighton, UK
CreditAttribution: ianthomas_uk commented
Status: Needs review » Reviewed & tested by the community
FileSize
2187829-views-check-plain-23.patch34.17 KB

The patch looks good now. It needed a really minor reroll because some of the context changed in ViewsUIController, but hopefully it's still OK to mark as RTBC.

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.