Add a token for the site logo

Thanks, let's hope we can get this on D7 as well (if soon)

@Neograph734 Haven't made any website on Drupal 8 yet, sorry.
(Cause Omega8.cc aren't supporting it yet :/ have any good, specialized & cheap hosting for europe? :D )

Review:

- the sole change in my patch is removing one piece of whitespace, so I'm marking this RTBC at the same time
- as @neograph734 says, the other site tokens are already in core, so see no objection for this to be
- patch applies cleanly on 8.3.x and works as described
- has automated tests, which I ran locally (I also ran a test-only patch and verified it failed)
- added issue summary template + steps to reproduce
- code style fine otherwise
- changes all in scope
- @neograph734 mentions doubts over the metadata in the IS, but has added the appropriate tests and it looks OK to me
- names of new tokens are sensible/consistent with what we already have
- this contains new translatable strings, tagged as string change
- no UI (CSS/JS) changes (other than the new text in token module - Available tokens window)

+++ b/core/modules/system/system.tokens.inc
@@ -127,6 +135,24 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
+          $logo_url = theme_get_setting('logo.url');
...
+          $logo_url = theme_get_setting('logo.url');

So if you're using Seven as your admin theme and you send an email from the admin interface it's going to use seven logo. Also what happens if the bartik logo is customised but not the global setting.

I think the only option is to have a logo per theme and the global one. But one thing that is interesting - theme_get_setting('logo.url') will return you the logo url of the active theme so I'm not sure that using theme_get_setting is right here.

What's for sure is that we need more testing of what happens when you have multiple themes with different logos.

Rerolled the patch, please review.

+++ b/core/modules/system/system.tokens.inc
@@ -149,6 +183,54 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
+          // If there is no logo key, we set the theme to NULL to use the global fallback.
...
+          // If there is no logo key, we set the theme to NULL to use the global fallback.

Line exceeding 80 characters

+++ b/core/modules/system/tests/src/Kernel/Token/TokenReplaceKernelTest.php
@@ -129,6 +129,106 @@ public function testSystemSiteTokenReplacement() {
+   * Tests the generation of site logo tokens
...
+    // Install Bartik

Comments should (noramlly) begin with a capital letter and end with a full stop / period .

Fixed and attached new patch.

#24 works great. I'd suggest to RTBC this. Can we get some more feedback?

Applied the patch from #24, still working on 8.4.2.

Yes I can confirm that too. #24 RTBC.

Thanks for your work here! I'm going to retest #24 against 8.5.x as feature requests should target the development branch. A release manager may consider backporting this to the production branch (8.4.x).

Image dimension test fails were artifacts of php container upgrade issues, unrelated to this patch.

+++ b/core/modules/system/system.tokens.inc
@@ -127,6 +135,32 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
+          $replacements[$original] = \Drupal::service('renderer')->renderPlain($build);

@@ -149,6 +183,56 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
+          $replacements[$original] = \Drupal::service('renderer')->renderPlain($build);

Should this render in a context, catch any bubbleable metadata and add to the existing one?

Good stuff! I found only a few minor things and a possible improvement:

1. +++ b/core/modules/system/system.tokens.inc
   @@ -36,6 +36,14 @@ function system_token_info() {
   +  $site['logo'] = [
   +      'name' => t("Logo"),
   +      'description' => t("The default site logo, or the logo of a specified theme."),
   +  ];
   +  $site['logo:url'] = [
   +      'name' => t("Logo URL"),
   +      'description' => t("The URL of the default site logo, or the logo of a specified theme."),
   +  ];
   

   Wrong indentation :)

2. +++ b/core/modules/system/system.tokens.inc
   @@ -149,6 +183,56 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +        $themes = \Drupal::service('theme_handler')->listInfo();
   ...
   +        $themes = \Drupal::service('theme_handler')->listInfo();
   

   These calls can be moved outside the foreach loop.

3. +++ b/core/modules/system/tests/src/Kernel/Token/TokenReplaceKernelTest.php
   @@ -129,6 +129,106 @@ public function testSystemSiteTokenReplacement() {
   +    // Generate and test tokens.
   +    $tests = [];
   +    $tests['[site:logo]'] = $global_logo;
   +    $tests['[site:logo:bartik]'] = $global_logo;
   +    $tests['[site:logo:not-enabled-theme]'] = '[site:logo:not-enabled-theme]';
   +    $tests['[site:logo-url]'] = $system_theme_global_config->get('logo.path');
   +    $tests['[site:logo-url:bartik]'] = $system_theme_global_config->get('logo.path');
   +
   ...
   +    $tests = [];
   +    $tests['[site:logo]'] = $theme_logo;
   +    $tests['[site:logo:bartik]'] = $theme_logo;
   +    $tests['[site:logo:not-enabled-theme]'] = '[site:logo:not-enabled-theme]';
   +    $tests['[site:logo-url]'] = $theme_config->get('logo.path');
   +    $tests['[site:logo-url:bartik]'] = $theme_config->get('logo.path');
   +    $tests['[site:logo-url:not-enabled-theme]'] = '[site:logo-url:not-enabled-theme]';
   

   Can we add the case of a non-default theme with a custom logo and check whether the token is correctly generated also in both of these cases?

4. +++ b/core/modules/system/tests/src/Kernel/Token/TokenReplaceKernelTest.php
   @@ -129,6 +129,106 @@ public function testSystemSiteTokenReplacement() {
   +      $this->assertEqual($output, $expected, new FormattableMarkup('System site information token %token replaced.', ['%token' => $input]));
   +      $this->assertEqual($bubbleable_metadata, $metadata_tests[$input]);
   ...
   +      $this->assertEqual($output, $expected, new FormattableMarkup('System site information token %token replaced.', ['%token' => $input]));
   +      $this->assertEqual($bubbleable_metadata, $metadata_tests[$input]);
   

   Deprecated calls, we should use ::assertEquals().

In all the four hunks of system.tokens.inc I'm seeing some duplicate code. I'm wondering whether we could factor it out to an internal helper function, something like this:

function _system_tokens_get_actual_theme($theme) {
  // Retrieve configurations.
  $config = \Drupal::config('system.theme.global');
  $bubbleable_metadata->addCacheableDependency($config);
  $theme_config = \Drupal::config($theme . '.settings');
  $bubbleable_metadata->addCacheableDependency($theme_config);

  // If there is no logo key, we set the theme to NULL to use the global
  // fallback.
  if (!$theme_config->get('logo.path')) {
    $theme = NULL;
  }

  return $theme;
}

In the first two cases we would be passing the default theme as parameter.

One more, sorry:

+++ b/core/modules/system/system.tokens.inc
@@ -36,6 +36,14 @@ function system_token_info() {
+  $site['logo'] = [
...
+  $site['logo:url'] = [

What about adding also info about all the theme-specific tokens?

Changes look good to me, thanks!

All other assertions in that file still use the old assertEqual(), I felt it would not be right to combine old and new standards, so I went ahead with what was already present in the file. If you still feel I should update it, I will do it.

I don't feel strongly about this, but it seems to me that our deprecation policy recommends not to introduce new usages of deprecated code, although it's not crystal clear on this matter:

Contributed project developers, as well as those maintaining custom integrations, should follow the deprecations when possible and use the latest APIs available.

I'm assuming that if it applies for contrib/custom code it should apply for core code as well.

I am not sure how to. Would it be something like this?

Well, I was thinking about explicitly listing the available theme-specific tokens.

To validate this suggestion, I enabled the Token module on a vanilla core installation and it seems to me that when values are known in advance the Tokens UI lists them explicitly. Question marks are used when the suffix in not specified or can be a custom string.

@Neograph734

Good effort!. But this patch has one bugs. Please refer below metnioed steps for testing scenario

1. Applied patch (2842780-50-token_for_site_logo.patch)
2. Go to Structure -> Content type -> Article
3. Click Manage fields -> Edit Body field
4. Click "Browse available tokens" underneath help text.
#BUG 1
Token displayed "[site:logo:?]" and "[site:logo-url:?]". It does not refer the theme name by default. Manually I have added the theme name in the token like "[site:logo:bartik]" & " [site:logo-url:bartik]". Then Logo and logo reference url - /drupal-8/core/themes/seven/logo.svg displayed great

#BUG 2
Incase I have omega theme it does not work like "[site:logo:omega]" & " [site:logo-url:omega]". Refer attached screenshots

I guess @ksujitha18 meant this :)

The RTBC status should be used when the patch is deemed ready for incorporation in the Drupal codebase.

@Neograph734,

#55, works only for the default site logo [site:logo] and site logo URL [site:logo-url].

Unable to use tokens like [site:logo:seven] or [site:logo-url:?]. Refer the screenshot

Please correct me if I am wrong.

Awesome... When I add dynamic to the system_token_info() function, it's working perfectly.

Can you add in the patch as well?

Patch attached

Hi Venkatesh

Can you please attach interdiff file?

Hi jeetendrakumar,

Interdiff file attached.

Thanks for reviewing...

1. +++ b/core/modules/system/system.tokens.inc
   @@ -127,6 +142,24 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +        case 'logo':
   +          $default_theme = \Drupal::config('system.theme')->get('default');
   +          $theme = _system_tokens_get_actual_theme($default_theme, $bubbleable_metadata);
   +          $logo_url = theme_get_setting('logo.url', $theme);
   

   I'm not sure if the default token should really use the default theme or if it should be using the *active* theme?

2. +++ b/core/modules/system/system.tokens.inc
   @@ -192,3 +256,28 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +
   +/**
   + * @param $theme
   + *   The theme to load the logo for.
   

   missing function description.

That is a valid point, but it will also use the *templates* of the active theme then, unless you use mailsystem and its setting for the mail theme.

If you send e-mails, you could explicitly specify the theme you want to have the token from. But if you e.g. use this token in metatags or so, then you'd want to have it respect the active theme?

One option would to always require an explicit selection and have site:logo:default and :active?

One of the use cases frequently mentioned above is to use the new tokens to include a locally-stored image in an email. This is something that is not possible since #1494670: References to CSS, JS, and similar files should be root-relative URLs: avoids mixed content warnings & fewer bytes to send went in (almost two years ago), because now all URLs generated by core for local assets are relative URLs. And there is no way in core to force them to be absolute. Images can't/won't be shown in an email unless you can produce an absolute URL with the token.

So +1 for putting this in, but only if you can give us absolute URLs because email is one of the primary needs for these tokens.

There is a proposal in #2704597: Relative URLs in mails should be converted to absolute ones to fix this relative/absolute issue for email, but there has been little activity over there. I think the tokens should wait until that issue or something like it is resolved, otherwise you're going to see a LOT of bug reports/support requests/unhappy users of these new tokens when they don't work as expected.

Didn't look at the code yet but I'm definitely +1 on the new approach. Thanks for putting it together!

I'm wondering whether default and active are reserved theme names or we could have some clashes in those cases...

Would it be possible to implement something like the following?

[site:logo:active-theme]
[site:logo:default-theme]
[site:logo:theme:bartik] or [site:logo:theme-bartik]

Yes, let's fix #2704597: Relative URLs in mails should be converted to absolute ones!

Nice work! I manually tested this with emails and the Token module and it works fine.

I found only a few mostly minor issues in code:

1. +++ b/core/modules/system/system.tokens.inc
   @@ -49,6 +59,40 @@ function system_token_info() {
   +    'description' => t("The logo of the activated theme. (Pay attention when using this from the administrative interface, as the theme might be different.)"),
   

   Shouldn't this say "active theme"? Also, the parentheses feel unnecessary to me.

2. +++ b/core/modules/system/system.tokens.inc
   @@ -49,6 +59,40 @@ function system_token_info() {
   +    'description' => t("The logo that is configured for the standard theme."),
   

   Shouldn't this say "default theme"?

3. +++ b/core/modules/system/system.tokens.inc
   @@ -49,6 +59,40 @@ function system_token_info() {
   +  $themes = \Drupal::service('theme_handler')->listInfo();
   

   We should filter out hidden themes (e.g. classy or stable), as those are usually base themes and are not available in the UI, so no logo can be configured.

4. +++ b/core/modules/system/system.tokens.inc
   @@ -147,6 +196,63 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +  }
   +
   +  elseif ($type == 'site-logo') {
   ...
   +  }
   +
   +  elseif ($type == 'site-logo-properties' && !empty($data['theme'])) {
   

   Surplus empty lines.

5. +++ b/core/modules/system/system.tokens.inc
   @@ -147,6 +196,63 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +      $name = (stripos($name, 'theme-') === 0) ? substr($name, 6) : $name;
   

   Does this mean we support also unprefixed names? These wouldn't be seen as valid tokens.

6. +++ b/core/modules/system/system.tokens.inc
   @@ -192,3 +298,51 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   + *   If set to TRUE, the logo will be rendered. An url is returned otherwise.
   

   Missing (optional) prefix and default description.

7. +++ b/core/modules/system/system.tokens.inc
   @@ -192,3 +298,51 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +  $logo_url = theme_get_setting('logo.url', $theme) ?: theme_get_setting('logo.url', $default_theme);
   

   Can we make this URL absolute, so that these tokens can be used in emails without waiting for #2704597: Relative URLs in mails should be converted to absolute ones?

8. +++ b/core/modules/system/tests/src/Kernel/Token/TokenReplaceKernelTest.php
   @@ -127,6 +127,161 @@ public function testSystemSiteTokenReplacement() {
   +    $tests['[site:logo:url]'] = '[site:logo:url]';
   

   Why aren't we supporting the global logo url?

9. +++ b/core/modules/system/tests/src/Kernel/Token/TokenReplaceKernelTest.php
   @@ -127,6 +127,161 @@ public function testSystemSiteTokenReplacement() {
   +      $output = $this->tokenService->replace($input, [], ['langcode' => $this->interfaceLanguage->getId()], $bubbleable_metadata);
   

   Do we need to specify the langcode here? If this is not necessary I'd leave it out, as it's confusing.

1: Just the brackets, thanks!
4: The existing space was not supposed to be there: https://www.drupal.org/docs/develop/standards/coding-standards#controlst... :)
5: Oh, I see, all good then
6: Optional parameters should always have a description of the default behavior, i.e. what happens when the parameter is not specified, if it's not obvious. In this case it would be something like: By default a URL is returned.
8: Got it, thanks

Thanks, we are very close to RTBC!

1. +++ b/core/modules/system/system.tokens.inc
   @@ -18,6 +19,16 @@ function system_token_info() {
   +    'name' => t("Site logo"),
   +    'description' => t("Tokens related to the site logo."),
   +    'needs-data' => 'site',
   

   (here and below) Can we stick to single quotes for consistency?

2. +++ b/core/modules/system/system.tokens.inc
   @@ -49,6 +60,42 @@ function system_token_info() {
   +  // Logo related tokens.
   +  $site_logo['active-theme'] = [
   

   [logo:url] is not defined in the token info.

3. +++ b/core/modules/system/system.tokens.inc
   @@ -192,3 +297,53 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +  $logo_url = theme_get_setting('logo.url', $theme) ?: theme_get_setting('logo.url', $default_theme);
   ...
   +  return Url::fromUserInput($logo_url, ['absolute' => TRUE])->toString();
   

   Can we store the absolute URL in $logo_url so that also the rendered image gets it?

4. +++ b/core/modules/system/system.tokens.inc
   @@ -192,3 +297,53 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +  // @todo: We might be able to use the relative $logo_url variable once https://www.drupal.org/project/drupal/issues/2704597 is complete.
   

   This comment does not wrap at column 80 properly.

I wouldn't touch the other token definitions, just the ones introduced by this patch. We just need to ensure the new code is consistent :)

1. +++ b/core/modules/system/system.tokens.inc
   @@ -192,3 +297,53 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +  $config = \Drupal::config('system.theme.global');
   +  $system_theme_config = \Drupal::config('system.theme');
   

   We need to add also these two to bubbleable metadata, I think, as global settings and the default theme may change, which would require cached logo token values to be invalidated.

2. +++ b/core/modules/system/system.tokens.inc
   @@ -192,3 +297,53 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +  $theme_config = \Drupal::config($theme . '.settings');
   +  $bubbleable_metadata->addCacheableDependency($config);
   +  $bubbleable_metadata->addCacheableDependency($theme_config);
   

   Now that I think about it, in the case of the active-theme we also need to manually add the theme cache context, otherwise if logo tokens are used in cacheable contexts, e.g. node bodies, the active theme will be cached and subsequent request may get the wrong theme.

#91 looks good to me, thanks!

Regarding #88-2, It is more or less in line with #80-8. site:logo renders the active theme logo, so site:logo:url should return the active theme url. But that also already had a dedicated token. I think it would be better to not actively promote it, as the dedicated tokens are more predictable.

If a token is not defined in token info, it cannot be validated, so it cannot be used wherever token validation is applied, e.g. user mail notification settings. If you strongly feel it shouldn't be promoted, then we should revert the change for #80.8 and not provide a replacement for it. The current state is confusing and inconsistent IMO :)

However that are a few things I ran into.

  $logo_url = Url::fromUserInput($logo_path, ['absolute' => TRUE])->toString();

  $build = [
    '#theme' => 'image',
    '#uri' => $logo_url,
  ];
  return \Drupal::service('renderer')->renderPlain($build);

This returns an image using the local path only: <img src="/path/to/file">, rather than <img src="http://example.com/path/to/file">

Yes, this is what I said in #73 above, as well as two years ago in the issue that broke this: #1494670-121: References to CSS, JS, and similar files should be root-relative URLs: avoids mixed content warnings & fewer bytes to send.

If you use '#theme' => 'image' you will ALWAYS get a relative path for a local image, even if you specify 'absolute' => TRUE.

The only way to get an absolute URL right now is to explicitly write out the link as markup:

  $logo_url = Url::fromUserInput($logo_path, ['absolute' => TRUE])->toString();

  $build = [
    '#markup' => '<img src="' . $logo_url . '" />',
  ];

This is not a great solution, because then you don't get the attributes in the img tag that would normally be added by the theme function, but at least you get a working link.

Please only tag string changes after the issue is actually committed. Thanks!

I was testing the patch #94 and faced an situation.

All my files (including Logo) are being saved on CDN. When I try to see the logo information replaced by token, the following message is delivered on my screen.
"This image has been removed. For security reasons, only images from the local domain are allowed."

Is it the behavior expected ?

Thank you all.

Patch #94 updated for Drupal 8.7

Patch applied on 8.8.

patch #94 applied and worked for D 8.6.16

The patch in #102 needs a re-roll for the 8.8.x branch. So I've set the "needs reroll" and "novice" tags for this task.

Adding tag for DCD 2019 code sprint.

Re-rolled patch for #102.

#2704597: Relative URLs in mails should be converted to absolute ones is fixed now (mentioned in patch #108 as comment).

#108 can't be RTBC because it doesn't even apply, "PHP 7.1 & MySQL 5.7 PHPLint Failed". You can't even try it out on your site without fixing this first.

The reason it fails to apply is a duplicate "use" statement in TokenReplaceKernelTest.php

Here is a new patch identical to #108 but with that duplicate "use" statement removed. I have NOT evaluated this patch to see if it solves the issue, this is just a re-roll to make the patch apply so the testbot can look at it and so it can be reviewed here.

Can/Should we also add a token for the admin theme?

Some nitpicks, mostly double quotes where we should use single quotes:

1. +++ b/core/modules/system/system.tokens.inc
   @@ -19,6 +19,16 @@ function system_token_info() {
   +    'name' => t("Site logo"),
   

   Use single quotes.

2. +++ b/core/modules/system/system.tokens.inc
   @@ -19,6 +19,16 @@ function system_token_info() {
   +    'description' => t("Tokens related to the site logo."),
   

   Use single quotes.

3. +++ b/core/modules/system/system.tokens.inc
   @@ -19,6 +19,16 @@ function system_token_info() {
   +    'name' => t("Site logo properties"),
   

   Use single quotes.

4. +++ b/core/modules/system/system.tokens.inc
   @@ -19,6 +19,16 @@ function system_token_info() {
   +    'description' => t("Tokens for site logo properties."),
   

   Use single quotes.

5. +++ b/core/modules/system/system.tokens.inc
   @@ -53,6 +63,42 @@ function system_token_info() {
   +    'description' => t('The logo of the active theme. Pay attention when using this from the administrative interface, as the theme might be different.'),
   

   The logo of the active theme. Note that the administration theme may have a different logo.

6. +++ b/core/modules/system/system.tokens.inc
   @@ -53,6 +63,42 @@ function system_token_info() {
   +        'description' => t('The logo for the %theme theme.', ['%theme' => $info->info['name']]),
   

   'description' => t('The logo for the %theme theme.', [
   '%theme' => $info->info['name'],
   ]),

7. +++ b/core/modules/system/system.tokens.inc
   @@ -196,3 +299,57 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +  if ($theme == 'default-theme') {
   

   ===

8. +++ b/core/modules/system/system.tokens.inc
   @@ -196,3 +299,57 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +  elseif ($theme == 'active-theme') {
   

   ===

I am confused ... what has troubled me a lot since ages was that the logo always relied on theme settings only, but not on basic site info configuration settings as a general override, where users type in the name and the slogan. So does this token will change the logo path regarding the theme used when the token is present? Doesn't it cause cache issues? Especially when the theme changes under certain conditions? What about contrib modules which change logo under certain conditions or themes which use more than one logo for different styles (mobile, etc.?)...

Hi @ TR

patch #110 file : 2842780-110.patch apply cleanly LGTM +1 RTBC .

Thanks..!!!

So does this token will change the logo path regarding the theme used when the token is present? Doesn't it cause cache issues? Especially when the theme changes under certain conditions? What about contrib modules which change logo under certain conditions or themes which use more than one logo for different styles (mobile, etc.?)...

I still think this considerations are not obsolete yet. Would love to get some thoughts on it to let it go ...

@#115: Thanks for reviewing and reporting. But... I copied the paragraph from the OT for you. Do you have done precisely what has been listed here before setting it RTBC? Please provide more details in your review. Thanks.

Steps to reproduce:
- without applying patch, install and enable 'token' module (so you can easily browse the list of available tokens)
- edit Article content type, go to manage fields, edit Body
- click "Browse available tokens" link underneath Help text, to open Available tokens popup
- verify no site logo tokens in the Site information list

- now apply patch
- drush cr
- reload Available tokens popup, verify [site:logo] and [site:logo-url] added
- insert them into help text and create or edit an article to verify the Drupal logo and standard URL (/core/themes/bartik/logo.svg) are now visible
- configure a different logo for another theme (seven)
- insert [site:logo:seven] and [site:logo-url:seven] to have the logo show up.
- [site:logo:non-existing-theme] should not be replaced

And for @#115: again: Apart from that please do not assign the issue to yourself while setting it RTBC. (It maybe happend accidently.) Just to let you know.

Thanks for proposing this feature. Since it is a new feature, I'm moving it to the 9.1.x development branch. I also queued a test against 9.1.x.

It looks like #111 could still be addressed. I also had a quick look at the patch myself and noticed a couple things. (Haven't done a full review.)

1. +++ b/core/modules/system/system.tokens.inc
   @@ -196,3 +299,57 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   + * @param $theme
   + *   The theme to load the logo for.
   

   This is missing a data type.

2. +++ b/core/modules/system/tests/src/Kernel/Token/TokenReplaceKernelTest.php
   @@ -2,11 +2,11 @@
   -use Drupal\Core\Url;
    use Drupal\Component\Render\FormattableMarkup;
    use Drupal\Component\Utility\Html;
    use Drupal\Component\Utility\Xss;
    use Drupal\Core\Render\BubbleableMetadata;
   +use Drupal\Core\Url;
   

   Out-of-scope change.

3. +++ b/core/modules/system/system.tokens.inc
   @@ -53,6 +63,42 @@ function system_token_info() {
   +    'description' => t('The logo of the active theme. Pay attention when using this from the administrative interface, as the theme might be different.'),
   

   "Pay attention" seems a bit adversarial. Maybe:

   Note that the theme may be different when this is used in the administrative interface.

4. +++ b/core/modules/system/system.tokens.inc
   @@ -53,6 +63,42 @@ function system_token_info() {
   +  // Theme specific logo tokens.
   

   Nit: Theme-specific.

5. +++ b/core/modules/system/tests/src/Kernel/Token/TokenReplaceKernelTest.php
   @@ -129,6 +129,188 @@ public function testSystemSiteTokenReplacement() {
   +   * @return array
   

   Can we more specifically describe the data type of the array? It is maybe array[] or string[][] or mixed[][] depending on the data structure of the returned config -- we could check the called method to see exactly how the data is structured.

Addressed #111 and #118
please review the patch.

Hello everyone!

I had the same issue with creating a token of site logo and tried to use patches from this topic. But into all of them use function theme_get_setting() and this function is deprecated in Drupal 8.7 and Drupal 9.
So I created new patch without function theme_get_setting(). I have tested this path with different themes and all ok for me.

Please, check my patch.

Applied the patch #122 on drupal 9.1x
Was able to see site logo token

If there is more code review needed, after that it can be moved to RTBC

@neograph that one looks better. Probably a code review is needed before we can move it to RTBC

Applied patch #133 .It works fine. There will be tokens available for site logo and site logo url after installing this patch.

Including screenshots
before patch:

after patch(tokens available):

after patch(tokens output):

RTBC

Fixing Drupal code standard.

1. +++ b/core/modules/system/tests/src/Kernel/Token/TokenReplaceKernelTest.php
   @@ -128,6 +128,146 @@ public function testSystemSiteTokenReplacement() {
   +    $tests['[site:logo]'] =
   +    $tests['[site:logo:active-theme]'] = $seven_logo;
   

   This looks really odd. Yes it is equivalent to

   $tests['[site:logo]'] = $tests['[site:logo:active-theme]'] = $seven_logo;
   

   but I don't think the changing it to this is really worth it.

   How about being explicit and doing...

     $tests['[site:logo]'] = $seven_logo;
     $tests['[site:logo:active-theme]'] = $seven_logo;
   

2. +++ b/core/modules/system/tests/src/Kernel/Token/TokenReplaceKernelTest.php
   @@ -128,6 +128,146 @@ public function testSystemSiteTokenReplacement() {
   +    $metadata_tests['[site:logo]'] =
   +    $metadata_tests['[site:logo:active-theme]'] =
   +    $metadata_tests['[site:logo:active-theme:url]'] = BubbleableMetadata::createFromObject($global_theme_config)
   +      ->addCacheableDependency($seven_config)
   +      ->addCacheContexts(['theme']);
   +    $metadata_tests['[site:logo:default-theme]'] =
   +    $metadata_tests['[site:logo:default-theme:url]'] = BubbleableMetadata::createFromObject($global_theme_config)
   +      ->addCacheableDependency($bartik_config)
   +      ->addCacheableDependency($system_theme_config);
   

   Ahhh I see you're doing the same here. I can see the benefit here but it still looks really odd. Do we have other examples of this format in core?

   Earlier in the same test we do

       $metadata_tests = [];
       $metadata_tests['[site:name]'] = BubbleableMetadata::createFromObject(\Drupal::config('system.site'));
       $metadata_tests['[site:slogan]'] = BubbleableMetadata::createFromObject(\Drupal::config('system.site'));
       $metadata_tests['[site:mail]'] = BubbleableMetadata::createFromObject(\Drupal::config('system.site'));
       $bubbleable_metadata = clone $base_bubbleable_metadata;
       $metadata_tests['[site:url]'] = $bubbleable_metadata->addCacheContexts(['url.site']);
       $metadata_tests['[site:url-brief]'] = $bubbleable_metadata;
       $metadata_tests['[site:login-url]'] = $bubbleable_metadata;
   

   Which, to me, looks as readable and less surprising than what's in the patch.

   The multiple assignments were introduced in #94 and have not yet been discussed in the comments as far as I can see.

   I'm not really sure... about this going to leave at RTBC and solicit more opinion.

Discussed with #138 with @catch and neither of us are particularly keen on multi-line multiple assignments. Let's see if there's another way of doing this.

I'm using a modified Bootstrap Barrio 8.x-4.28 theme. in the registration e-mail of the new user the website logo should appear but it looks:

"

img src="/sites/default/files/logo2.0_1.png" alt="" typeof="foaf:Image"

AnnaEjankowska

Dziękujemy za rejestrację na stronie e-jankowska. Twoja prośba o utworzenie konta oczekuje na zatwierdzenie. Po zatwierdzeniu otrzymasz kolejny e-mail zawierający informacje o tym, jak się zalogować, ustawić hasło i inne szczegóły.

-- Zespół e-jankowska
"
instead of the displayed image.
I use the swift mailer to send e-mails.
core drupal 9.2 PHP 7.4
a screenshot of the settings for shipping.
How to fix this issue?

(You need to format your post with the "code" formatting so drupal.org displays the raw output without the HTML tags escaped.)

@AnnaE1990: Drupal core does not directly support HTML email, that functionality is provided by contributed modules. If your email is not displaying the HTML correctly that's a problem with the contributed module you're using (swiftmailer). That's unrelated to this issue. If you type in an image tag directly instead of using the token you would see the same result.

This issue is about making the token available and making sure the token has the right value. I think your post is saying the token has the value <img src="/sites/default/files/logo2.0_1.png" alt="" typeof="foaf:Image">, which appears to be basically correct except it's a relative URL instead of absolute. I raised this as a major problem more than 3 years ago in #73, and I'm disappointed that this problem hasn't been addressed yet.

there is a fresh module, and its working fine https://www.drupal.org/project/simple_html_mail

+++ b/core/modules/system/system.tokens.inc
@@ -196,3 +301,52 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
+ * @param bool $render
+ *   (Optional) If this is set to TRUE, this function will return a rendered
+ *   logo. It will return an URL to the logo otherwise.

This is powerful.

Is there a way we can make this a more systemic change?

(Note: I'm still familiarising myself with the deeper token sysetms)

To push things forward here and as @alexpott's comments from #138 have been addressed in #145 settings this RTBC.

Any further thoughts or issues? Otherwise, I think we could try to finish this nice feature after > 5 years? :)

LGTM!

Thanks @Neograph734, yes indeed. Could you please include an interdiff to be sure? Thanks!

1. +++ b/core/modules/system/system.tokens.inc
   @@ -50,6 +60,44 @@ function system_token_info() {
   +  // Token [site:logo] renders the active theme logo. Nesting for more options.
   

   What does "Nesting for more options." mean?

2. +++ b/core/modules/system/system.tokens.inc
   @@ -152,10 +202,65 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +      $name = (stripos($name, 'theme-') === 0) ? substr($name, 6) : $name;
   

   $name = str_starts_with($name, 'theme-') ? substr($name, 6) : $name;

   Easier to read and with have PHP 8 polyfill on D9.

3. +++ b/core/modules/system/system.tokens.inc
   @@ -197,3 +302,52 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
   +  if ($render) {
   +    $build = [
   +      '#theme' => 'image',
   +      '#uri' => $logo_url,
   +    ];
   +    return \Drupal::service('renderer')->renderPlain($build);
   +  }
   +  return Url::fromUserInput($logo_url, ['absolute' => TRUE])->toString();
   

   I think we should have one function to get the logo url for the provided theme and another function to get the rendered logo.

   I'm also not convinced that we should renderPlain here. I think we want a markup object so the token system knows this is markup and does the right thing with it in different contexts. See #2580723: Fix token system confusion, with new function Token::replacePlain() for more about this.

   I also wonder what we should do for accessibility. I think the rendered image should have alt text.

+++ b/core/modules/system/system.tokens.inc
@@ -197,3 +302,52 @@ function system_tokens($type, $tokens, array $data, array $options, BubbleableMe
+function _system_tokens_get_actual_logo($theme, BubbleableMetadata &$bubbleable_metadata, $render = FALSE) {

We should use primitive typehints here...
_system_tokens_get_actual_logo(string $theme, BubbleableMetadata &$bubbleable_metadata, bool $render = FALSE) {

Also the _actual in the function name does seem to add anything to me.

Also See above - I think this function should be split into two functions so the return value is consistent.

Plus we should be using return typehints too.

@alexpott i add primitive typehints in _system_tokens_get_actual_logo function as per your comment #162.
But other changes like function should be split into two functions so the return value is consistent.
Plus we should be using return typehints still remains.