There's no more String class in core after #2458387: Remove Utility\String class
And tests are outdated

CommentFileSizeAuthor
#5 string-safemarkup-4.patch12.35 KBandypost
#5 interdiff.txt9.54 KBandypost
#2 string-safemarkup-1.patch7.09 KBandypost
#2 interdiff.txt6.32 KBandypost
string-safemarkup.patch957 bytesandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, string-safemarkup.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
interdiff.txt6.32 KB
string-safemarkup-1.patch7.09 KB

Tests should be cleaned-up too

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm at OSInet commented

This looks good, but I noticed duplicated code (which already existed, actually). This line is present in all tests, and it would be nicer if it were refactored to just a single occurrence, say in a XmlRpcTestBase

    $url = \Drupal::url('xmlrpc.php', [], ['absolute' => TRUE]);
fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm at OSInet commented
Status: Needs review » Needs work
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost at Skilld commented
Status: Needs work » Needs review
FileSize
interdiff.txt9.54 KB
string-safemarkup-4.patch12.35 KB

So looking at basic_auth core module tests, better to use trait

  • fgm committed 74c13c4 on 8.x-1.x authored by andypost 
    Issue #2486179 by andypost: convert String to SafeMarkup.
fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm at OSInet commented
Status: Needs review » Fixed

Committed to 8.x-1.x, thank you.

Status: Fixed » Closed (fixed)

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