Part of #2557113: Make t() return a TranslationWrapper object to remove reliance on a static, unpredictable safe list

Problem/Motivation

In \Drupal\Core\Database\Install\Tasks, we currently do:

$this->results[$message] = FALSE;

This is problematic because $message is output from t(), which may output an object at one point, and PHP does not allow storing objects in array keys.

Proposed resolution

Put the message in the array value instead.

Remaining tasks

Commit

User interface changes

No

API changes

None.

Data model changes

N/A

CommentFileSizeAuthor
#2 2560719-1.patch1.17 KBstefan.r
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stefan.r created an issue. See original summary.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Active » Needs review
FileSize
2560719-1.patch1.17 KB
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Priority: Normal » Major
Issue tags: +Needs manual testing

Split from a major so I'm bumping this as well. Also this patch looks great.

Needs likely a manual tests (install screen uses tasks I think and after installing a module through the UI) IIRC

Hopefully I get a free moment to test this out today, but anybody feel free to review.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Issue summary: View changes
stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Parent issue: » #2557113: Make t() return a TranslationWrapper object to remove reliance on a static, unpredictable safe list
stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Issue tags: -Needs manual testing

Seems this array is only used to print out the failure messages when we do $this->fail() from tasks, so did a manual install where I triggered 2 failures and a pass:

Confirmed this should work the same for mysql/postgres/sqlite, having reviewed their implementations of Tasks.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Needs review » Reviewed & tested by the community

I manually tested - works a treat.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Issue summary: View changes
catch’s picture

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

Committed/pushed to 8.0.x, thanks!

  • catch committed f2194d1 on 8.0.x 
    Issue #2560719 by stefan.r: Remove usage of t() string in the array keys...

Status: Fixed » Closed (fixed)

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