Prevent escaping of other patterns included in share

[remydenton]

Jira

http://vjira2:8080/browse/WWWD-2454

Summary

Prevent auto-escaping of patterns included within the Share component

Details

Auto-escaping is not enabled in Pattern Lab, so you won't see this issue on the PL side.

Auto-escaping is enabled on the Drupal side, as it should be for security. However, one of the side affects of the way the escaping is done is that if you set a variable to the result of an include function, the output will be escaped HTML. In the example below, link1 will be escaped HTML, while link2 and link3 will print as rendered links

{% set link1 = include("@bolt-components-link/link.twig", {
  text: "Link 1",
  url: "#!"
}) %}

{% set link2 %}
  {{ include("@bolt-components-link/link.twig", {
    text: "Link 2",
    url: "#!"
  }}
}) %}

{% set link3 %}
  {% include "@bolt-components-link/link.twig" with {
    text: "Link 3",
    url: "#!"
  } only %}
}) %}

{{ link1 }}
{{ link2 }}
{{ link3 }}

This may actually be a bug in how Drupal's auto-escaping works since the result of an include function should be considered safe (as the examples for link2 and link3 above demonstrate). But, this PR gets the job done. If we choose this route, the rule of thumb is to never save the result of include() to a variable without printing it in Bolt components (use the technique for either link2 or link3 instead). The syntax for link1 is fine to use in Pattern Lab.

How to test

Reproduce the bug

• In Drupal, include a bare bones share component in a twig template

{% include '@bolt-components-share/share.twig' with {
  inline: true,
  text: 'Share this page'|trans,
  sources: [
    {
      name: "facebook",
      url: "#!"
    }
} only %}

• Disable javascript
• Confirm that escaped markup is output to the page

Fix the bug

• Copy the updated markup for share.twig from this PR into Drupal
• Refresh the page and confirm that the markup is not escaped

[bolt-bot]

⚡ PR built on Travis and deployed a now preview here:

• Branch link: https://fix-WWWD-2454-include-patterns-in-share-without-escaping.bolt-design-system.com
• Permalink: https://bolt-design-system-axczocmtcj.now.sh

• Commit built: 8862af0
• Travis build

[bolt-bot]

⚡ Sauce Labs Test for chrome Passed!

Test Details

• Browser Name: Chrome
• Browser Version: 47.0.2526.73
• Browser Platform: Mac OS X
• View Test in Sauce Labs

[bolt-bot]

⚡ Sauce Labs Test for internet explorer Passed!

Test Details

• Browser Name: Internet explorer
• Browser Version: 11
• Browser Platform: WINDOWS
• View Test in Sauce Labs

[bolt-bot]

⚡ Sauce Labs Test for chrome Passed!

Test Details

• Browser Name: Chrome
• Browser Version: 47.0.2526.73
• Browser Platform: Mac OS X
• View Test in Sauce Labs

[bolt-bot]

⚡ Sauce Labs Test for internet explorer Passed!

Test Details

• Browser Name: Internet explorer
• Browser Version: 11
• Browser Platform: WINDOWS
• View Test in Sauce Labs

[sghoweri]

@remydenton out of curiosity, would we be seeing this exact same issue in Pattern Lab / on the docs site if we enabled autoescaping in our Twig environment? Just wondering if enabling that would help prevent issues like these from popping up.

The other thing I’m wondering: is this problem you’re outlining here happening in any other Bolt components as far as we know?

[remydenton]

@sghoweri, both good questions.

1. I tried enabling autoescape in pattern lab here: https://github.com/bolt-design-system/bolt/blob/master/apps/pattern-lab/config/config.yml#L56. It escapes lots of things, to the point where it seemed unfeasible to update (and probably wouldn't match Drupal's behavior in any case-- see https://www.drupal.org/project/drupal/issues/1825952 for an idea of the subtleties involved).

2. No, there's no other place in Bolt components that sets a variable with the include() function before printing. There are places where the results of include() are printed directly, which works fine. For example:
   https://github.com/bolt-design-system/bolt/blob/master/packages/components/bolt-band/src/band--flag.twig#L27.
   https://github.com/bolt-design-system/bolt/blob/master/packages/components/bolt-logo/src/logo.twig#L6

[remydenton]

On further thought, maybe Pattern Lab doesn't necessarily need to match Drupal autoescaping since they are both just consumers of Bolt. On the other hand, Drupal Lab, if we ever have the bandwidth to get it up and running, would be a better place to test for this kind of thing.

[sghoweri]

@remydenton marking this as approved per the chat we just had on this -- you can go ahead and merge this in, however we'll still need to work on a broader solution in PL to address the environment differences between here and Drupal