CommentFileSizeAuthor
#4 fix_double_escaping_of-2531652-3.patch2.06 KBcilefen

Comments

hass’s picture

hass commented

Not sure if this is safe:

use Drupal\Component\Utility\SafeMarkup;

    $form['tracking']['domain_tracking']['google_analytics_domain_mode'] = [
      '#type' => 'radios',
      '#title' => t('What are you tracking?'),
      '#options' => [
        0 => SafeMarkup::set(t('A single domain (default)') . '<div class="description">' . t('Domain: @domain', ['@domain' => $_SERVER['HTTP_HOST']]) . '</div>'),
        1 => SafeMarkup::set(t('One domain with multiple subdomains') . '<div class="description">' . t('Examples: @domains', ['@domains' => implode(', ', $multiple_sub_domains)]) . '</div>'),
        2 => SafeMarkup::set(t('Multiple top-level domains') . '<div class="description">' . t('Examples: @domains', ['@domains' => implode(', ', $multiple_toplevel_domains)]) . '</div>'),
      ],
      '#default_value' => $config->get('domain_mode'),
    ];
hass’s picture

hass commented
Parent issue: » #2501975: Determine how to update code that currently joins strings in SafeMarkup::set()
hass’s picture

hass commented

Looks not better:

    $form['tracking']['domain_tracking']['google_analytics_domain_mode'] = [
      '#type' => 'radios',
      '#title' => t('What are you tracking?'),
      '#options' => [
        0 => SafeMarkup::format(t('A single domain (default)') . '<div class="description">' . t('Domain: @domain', ['@domain' => $_SERVER['HTTP_HOST']]) . '</div>'),
        1 => SafeMarkup::format(t('One domain with multiple subdomains') . '<div class="description">' . t('Examples: @domains', ['@domains' => implode(', ', $multiple_sub_domains)]) . '</div>'),
        2 => SafeMarkup::format(t('Multiple top-level domains') . '<div class="description">' . t('Examples: @domains', ['@domains' => implode(', ', $multiple_toplevel_domains)]) . '</div>'),
      ],
      '#default_value' => $config->get('domain_mode'),
    ];
cilefen’s picture

cilefen commented
Status: Active » Needs review
StatusFileSize
new fix_double_escaping_of-2531652-3.patch2.06 KB

SafeMarkup::set() is for internal use only.

cilefen’s picture

cilefen commented

None of this is user-entered in this form section so there is little to worry about.

hass’s picture

hass commented
Status: Needs review » Fixed

Translatable strings could be user entered on l.d.o :-)

  • hass committed 22a8ac3 on 8.x-2.x authored by cilefen 
    Issue #2531652 by cilefen: Fix double escaping of domain_mode radios
cilefen’s picture

cilefen commented

@hass Ah, yes. This is true.

cilefen’s picture

cilefen commented

Now I am less sure of myself. I think that in SafeMarkup::format() we may need to use replacement tokens to be safe.

hass’s picture

hass commented

Made a better change. http://drupalcode.org/project/google_analytics.git/commit/3529d9e

hass’s picture

hass commented
Related issues: +#2531656: Allow adding optional '#description' to options of radios and checkboxes
cilefen’s picture

cilefen commented

Nice! I didn't know about that option #description either.

hass’s picture

hass commented

Me too...

Status: Fixed » Closed (fixed)

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