It's a one-liner fix.

Wrong:
t('The following From address failed: !from', array('!from' => $smtp_from)),

Right:
t('The following From address failed: @from', array('@from' => $smtp_from)),

See also:
https://www.drupal.org/node/2575819

CommentFileSizeAuthor
#24 fixes_t_arguments-2843350-24.patch2.87 KBwundo
#7 smtp_authentication_support-wrong_t_placeholder-2843350-D8.patch2.87 KBmgoncalves
#2 0001-Issue-2843350-by-estoyausente-wrong-t-placeholder-us.patch3.22 KBestoyausente
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cppg created an issue. See original summary.

estoyausente’s picture

estoyausente
Primary language Spanish
CreditAttribution: estoyausente as a volunteer commented
Status: Active » Needs review
FileSize
0001-Issue-2843350-by-estoyausente-wrong-t-placeholder-us.patch3.22 KB

Changed all !var placeholders and replaced with @var in t functions.

cppg’s picture

cppg CreditAttribution: cppg commented

It's not ok, I'm afraid. @!smail is not a correct syntax, it should be @smail.
First 4 lines are not ok, last 2 are ok.

estoyausente’s picture

estoyausente
Primary language Spanish
CreditAttribution: estoyausente as a volunteer commented
Status: Needs review » Needs work

Yo're right!. I will be do asap.

pawel_r’s picture

pawel_r CreditAttribution: pawel_r commented

Thx #2.
After applying this patch e-mails from Contact Forms are being send properly.
Drupal 8.2

mgoncalves’s picture

mgoncalves CreditAttribution: mgoncalves at CI&T commented
Assigned: Unassigned » mgoncalves
mgoncalves’s picture

mgoncalves CreditAttribution: mgoncalves at CI&T commented
FileSize
smtp_authentication_support-wrong_t_placeholder-2843350-D8.patch2.87 KB

The issue reported was fixed, but I have a question about the t() function.
In this case, should I use the $this->t() or keep only t() ?

mgoncalves’s picture

mgoncalves CreditAttribution: mgoncalves at CI&T commented
Assigned: mgoncalves » Unassigned
Status: Needs work » Needs review
gg24’s picture

gg24 CreditAttribution: gg24 as a volunteer and at QED42 commented

Hi @mgoncalves,

It should be this->t() as t() has been deprecated and will be removed in Drupal 9.

Thanks!

gg24’s picture

gg24 CreditAttribution: gg24 as a volunteer and at QED42 commented
Status: Needs review » Needs work
mgoncalves’s picture

mgoncalves CreditAttribution: mgoncalves at CI&T commented
Assigned: Unassigned » mgoncalves
estoyausente’s picture

estoyausente
Primary language Spanish
CreditAttribution: estoyausente as a volunteer commented

@mgoncalves you cannot use $this->t because the code inside SMTP class is an external library, not a real Drupal class. Set inside this file a Drupal t function is a dirty way of resolve a problem that have to be resolve including the original library using composer or something similar.

@see https://www.drupal.org/node/2711559

mgoncalves’s picture

mgoncalves CreditAttribution: mgoncalves at CI&T commented

Hello @estoyausente,

I thought about that, Drupal 8 is keeping some hooks, and we cannot use $this->t inside hooks as well, do you ?
At this bug, the only thing I have seen is about the change from ! to @.

Do you have any point to add or the last patch I sent is satisfacting ?

estoyausente’s picture

estoyausente
Primary language Spanish
CreditAttribution: estoyausente as a volunteer commented

The last past seems perfect but (viewing the code) but I haven't enough time to test in in my local environment. For this reason I don't change the issue status to RTBC :-)

estoyausente’s picture

estoyausente
Primary language Spanish
CreditAttribution: estoyausente as a volunteer commented
Status: Needs work » Needs review
gg24’s picture

gg24 CreditAttribution: gg24 as a volunteer and at QED42 commented

Hi @mgoncalves,

Your patch works as intended.

Thanks!

gg24’s picture

gg24 CreditAttribution: gg24 as a volunteer and at QED42 commented
Status: Needs review » Reviewed & tested by the community
mgoncalves’s picture

mgoncalves CreditAttribution: mgoncalves at CI&T commented
Assigned: mgoncalves » Unassigned
arnoldbird’s picture

arnoldbird CreditAttribution: arnoldbird commented

Looks like this patch has not been applied to the dev branch yet.

arnoldbird’s picture

arnoldbird CreditAttribution: arnoldbird commented
Version: 8.x-1.0-beta2 » 8.x-1.x-dev
arnoldbird’s picture

arnoldbird CreditAttribution: arnoldbird commented

The patch in #7 is working for me. I no longer get the relevant error when I apply the patch to the latest dev code.

I can't send emails yet, but I think that's due to an unrelated issue.

wundo’s picture

wundo CreditAttribution: wundo at Chuva Inc. commented

Patch needs a re-roll, I'm going to manually fix it and commit.

wundo’s picture

wundo CreditAttribution: wundo at Chuva Inc. commented
wundo’s picture

wundo CreditAttribution: wundo at Chuva Inc. commented
Status: Reviewed & tested by the community » Needs review
FileSize
fixes_t_arguments-2843350-24.patch2.87 KB

Here it goes!

  • wundo committed 35c4555 on 8.x-1.x 
    Issue #2843350: Fixes improper t() usage inside AddAnAddress method
  • wundo committed 88213fd on 8.x-1.x
    Issue #2843350: Fixes improper t() usage
  • wundo committed aced43d on 8.x-1.x
    Issue #2843350: Fixes improper t() usage inside SetFrom method
wundo’s picture

wundo CreditAttribution: wundo at Chuva Inc. commented
Status: Needs review » Reviewed & tested by the community

As everything passed, I think this new patch is good to go :)

  • wundo committed 66a7035 on 8.x-1.x 
    Issue #2843350 by mgoncalves, estoyausente, wundo, gg24, cppg,...
wundo’s picture

wundo CreditAttribution: wundo at Chuva Inc. commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

pianomansam’s picture

pianomansam CreditAttribution: pianomansam commented

Can we can a full release with this fix? It's been 12 months since it was committed to dev.

lukus’s picture

lukus
he/him
CreditAttribution: lukus commented

+1 for release