Hello!

I have the fatal error after creating messages (notifications):

The website encountered an unexpected error. Please try again later.
Error: Call to undefined method Drupal\Core\StringTranslation\TranslatableMarkup::toString() in Drupal\socialbase\Plugin\Preprocess\Activity->preprocess() (line 28 of profiles/contrib/social/themes/socialbase/src/Plugin/Preprocess/Activity.php).

How to reproduce:

1) The function "template_preprocess_activity()" has the hext strings (see 39th line):

  $date = t('%time ago', ['%time' => $created_time_ago->getString()]);
  if ($full_url == '') {
    $variables['date'] = $date;
  }
  else {
    $variables['date'] = Link::fromTextAndUrl($date, $full_url);
  }

2) "Activity->preprocess()" has the next strings (see 27th line):

  // Remove href from date.
  $variables['date'] = strip_tags($variables['date']->toString()->getGeneratedLink());

But it works correctly only if we have Link in $variables['date']. When we have TranslatableMarkup in the variable, it generates fatal error because TranslatableMarkup has not "toString()" method (it has "__toString()" only).

CommentFileSizeAuthor
#10 social-fix_fatal_tostring-2956971-10.patch1.73 KBmikhailkrainiuk
#9 2956971-activity-notifications-box.png252.94 KBjaapjan
#8 social-fix_fatal_tostring-2956971-8.patch1.72 KBmikhailkrainiuk
#7 social-fix_fatal_tostring-2956971-7.patch1.35 KBmikhailkrainiuk
#6 2956971-activitystream.png67.24 KBjaapjan
#2 social-fix_fatal_tostring-2956971-2.patch1.38 KBmikhailkrainiuk

Comments

mikhailkrainiuk created an issue. See original summary.

mikhailkrainiuk’s picture

mikhailkrainiuk commented
Status: Active » Needs review
StatusFileSize
new social-fix_fatal_tostring-2956971-2.patch1.38 KB

I created the patch to fix it.
It works fine for me. Verify it, please.

jaapjan’s picture

jaapjan commented

Thanks for the report and the patch.

Looks like there is a similar issue here which also describes the issue and has a patch:
https://www.drupal.org/files/issues/social-fix-fatal-error-for-activitie...

What do you think would be the best way to solve it?

mikhailkrainiuk’s picture

mikhailkrainiuk commented

Hmm.
I checked the patch from this issue.
As I see, $variables['date'] will have different types - string or object, which needs special methods before rendering.
The patch from the current issue creates $variables['date'] with renderable data.

And 30th string has a strange code:

if ($variables['date'] instanceof TranslatableMarkup) {
  $variables['date'] = strip_tags($variables['date']->toString()
  ->getGeneratedLink());
}

TranslatableMarkup doesn't need special method for rendering string and can't build "getGeneratedLink". Maybe author wanted to check "instanceof Link"?

Well, looks like the patch from current issue is better. But let's see other opinions :)

jaapjan’s picture

jaapjan commented

I actually follow your logic and agree with you. Opened a PR for this on GH: https://github.com/goalgorilla/open_social/pull/818

jaapjan’s picture

jaapjan commented
Status: Needs review » Needs work
StatusFileSize
new 2956971-activitystream.png67.24 KB

Actually I've just checked it, even though the tests are passing, the functionality does not work correctly yet on default Open Social installation.

See this screenshot and you'll notice that the output is printed as HTML.
HTML showing in image

Marking it as "Needs work" for now.

mikhailkrainiuk’s picture

mikhailkrainiuk commented
Status: Needs work » Needs review
StatusFileSize
new social-fix_fatal_tostring-2956971-7.patch1.35 KB

Sure. We have strip_tags(), it changes HTML link.

If we build plain text "5 days ago", it remains plain.
If we build a link, it doesn't need "strip_tags" command.
I created another patch to fix it. Verify it, please.

mikhailkrainiuk’s picture

mikhailkrainiuk commented
StatusFileSize
new social-fix_fatal_tostring-2956971-8.patch1.72 KB

Hmm. We have a problem with Link::toString() - it makes HTML to plain text.
I updated the patch to fix it.

P.S. we use "@time ago" instead of "%time ago" because "%" generates inner HTML in link (new "em" tag). We have a problem with Link with inner HTML (see https://www.drupal.org/node/2392803).
But HTML inside link is not required in the current case.

jaapjan’s picture

jaapjan commented
Status: Needs review » Needs work
StatusFileSize
new 2956971-activity-notifications-box.png252.94 KB

The activity streams and node/post/comment page appears to be working now. But there is a new issue in the activity items now in the block in the header. See screenshot:
HTML is not allowed

Appears to be because the removed line in themes/socialbase/src/Plugin/Preprocess/Activity.php

Not sure how to solve that in a flexible way. Any ideas?

mikhailkrainiuk’s picture

mikhailkrainiuk commented
Status: Needs work » Needs review
StatusFileSize
new social-fix_fatal_tostring-2956971-10.patch1.73 KB

We have the problem with double links:
File social/themes/socialbase/templates/activity/activity--notification.html.twig has code:

<a href="{{ full_url }}" class="{{ status_class }}">
  <div{{ attributes.addClass('media') }}>
    <div class="media-left">
      {% if actor %}
        {{- actor -}}
      {% endif %}
    </div>
    <div class="media-body">
      {% if content %}
        {{- content -}}
      {% endif %}
      <div class="text-gray-light">{{ date }}</div>
    </div>
  </div>
</a>

We have link in {{ date }} inside.
But this code is wrapped in the link too.

Browsers get the conflict: 

<a href="link1">
  First link
  <a href="link2">second link</a>
</a>

Tag "a" inside tag "a".
If we have a link in the template, I suggest removing the link from the date. I updated the patch.
I tested it on the local website with Social - http://prntscr.com/j2u04n
HTML is rendered correct now and I have links for each notification.

jaapjan’s picture

jaapjan commented

Ah, I see. Yes, browsers don't like that! I personally think the link to the original entity on the date is an important feature we should not remove. I suggest to check for view_mode notification or notification_archive instead:

https://github.com/goalgorilla/open_social/pull/818/commits/ced9a1223c4b...

jaapjan’s picture

jaapjan commented
Status: Needs review » Fixed

Issue is merged and will be in the next minor release (1.17 is planned). Thanks!

Status: Fixed » Closed (fixed)

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