Problem/Motivation

core/modules/field_ui/src/FieldStorageConfigListBuilder calls SafeMarkup::set() which is meant to be for internal use only.

Proposed resolution

  • Remove the call by refactoring the code.

Remaining tasks

  1. Evaluate whether the string can be refactored to one of the formats outlined in this change record: https://www.drupal.org/node/2311123 Approach should be the same as #2505931: Remove SafeMarkup::set in ViewListBuilder (currently adding '#theme' => 'inline_list'
  2. Identify whether there is existing automated test coverage for the sanitization of the string. If there is, list the test in the issue summary. If there isn't, add an automated test for it. The function concatenates known-safe strings and does not need to be individually sanitized.

Manual testing steps

  1. Install Drupal 8 using the Standard profile.
  2. Log in as the admin user
  3. Visit admin/reports/fields
  4. Compare the "Used In" column of the Field List report, specifically for the body field which should be a comma-separated list

User interface changes

N/A

API changes

N/A

Files: 
CommentFileSizeAuthor
#18 2501971.18.patch1.11 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,862 pass(es). View
#18 13-18-interdiff.txt645 bytesalexpott
#15 2501971-13.png220.63 KBpjonckiere
#15 2501971-head.png238.51 KBpjonckiere
#13 2501971-9.patch1.1 KBstefan.r
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,634 pass(es), 2 fail(s), and 0 exception(s). View
#13 2501971-9.patch1.1 KBstefan.r
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,633 pass(es), 2 fail(s), and 0 exception(s). View
#8 2501971-8.patch1.05 KBstefan.r
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,637 pass(es), 2 fail(s), and 0 exception(s). View

Comments

tetranz’s picture

tetranz’s picture

Priority: Normal » Major
joelpittet’s picture

Status: Active » Postponed
tetranz’s picture

Assigned: tetranz » Unassigned

Unassigning myself for now until there is a clear direction for 2501975 and I have more time.

akalata’s picture

Status: Postponed » Active

No longer blocked, though the discussion in #2501975: Determine how to update code that currently joins strings in SafeMarkup::set() might be valuable to identify the best solution in this case.

akalata’s picture

Status: Active » Postponed
Related issues: +#2505931: Remove SafeMarkup::set in ViewListBuilder

Postponed for real on #2505931: Remove SafeMarkup::set in ViewListBuilder. Whatever pattern we find works there can most likely be used here as well.

akalata’s picture

Title: Remove or document SafeMarkup::set in FieldStorageConfigListBuilder » [PP-1] Remove or document SafeMarkup::set in FieldStorageConfigListBuilder
Issue summary: View changes
stefan.r’s picture

FileSize
1.05 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,637 pass(es), 2 fail(s), and 0 exception(s). View
Wim Leers’s picture

Title: [PP-1] Remove or document SafeMarkup::set in FieldStorageConfigListBuilder » Remove or document SafeMarkup::set in FieldStorageConfigListBuilder
Status: Postponed » Needs review
dawehner’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/core/modules/field_ui/src/FieldStorageConfigListBuilder.php
@@ -119,13 +118,10 @@ public function buildRow(EntityInterface $field_storage) {
+    $row['data']['usage'] = [
+      '#theme' => 'inline_list',
+      '#items' => $usage,
+    ];

This is also not performance critical ... so having a render array is perfect here.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: 2501971-8.patch, failed testing.

stefan.r’s picture

This needs to be updated still

stefan.r’s picture

Status: Needs work » Needs review
Issue tags: +Needs manual testing
FileSize
1.1 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,633 pass(es), 2 fail(s), and 0 exception(s). View
1.1 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,634 pass(es), 2 fail(s), and 0 exception(s). View
stefan.r’s picture

Title: Remove or document SafeMarkup::set in FieldStorageConfigListBuilder » Remove SafeMarkup::set in FieldStorageConfigListBuilder
pjonckiere’s picture

Status: Needs review » Needs work
Issue tags: -Needs manual testing
FileSize
238.51 KB
220.63 KB

This doesn't seem to work:

The last submitted patch, 13: 2501971-9.patch, failed testing.

The last submitted patch, 13: 2501971-9.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
645 bytes
1.11 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,862 pass(es). View

Let's fix it then. Got to love how #type => 'table' works :)

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

Got to love how #type => 'table' works :)

Yeah…

stefan.r’s picture

Yep, manually tested in admin/reports/fields and this seems to work great.

joelpittet’s picture

FYI, if you put a 'data' key that is #theme => table's preprocess thing, #type => table treats the nested keys as elements.

*star rainbow*

https://www.drupal.org/node/1876710

catch’s picture

Status: Reviewed & tested by the community » Fixed

Diff looks much nicer.

Committed/pushed to 8.0.x, thanks!

  • catch committed 8540c2f on 8.0.x
    Issue #2501971 by stefan.r, alexpott: Remove SafeMarkup::set in...

Status: Fixed » Closed (fixed)

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