Add support for attributes in backdrop_add_css().

[herbdool]

Description of the need

We should allow for attributes like integrity, and crossorigin to be passed to backdrop_add_js and backdrop_add_css. [Update: this was half done, it turns out.]

There's this long standing Drupal issue https://www.drupal.org/project/drupal/issues/1664602 where the latest patches seem to be working well.

I'm feeling like this issue already exists here but I just can't find it.

[indigoxela]

Hm... Looking at the api page, I see:

attributes: An associative array of attributes for the <script> tag. This may be used to add 'async' or custom attributes. Note that setting any attributes will disable preprocessing as though the 'preprocess' option was set to FALSE.

Are you sure, that attributes aren't possible?

BTW: I'd reduce that to backdrop_add_js() - or is there any need for additional attributes on <style> tags?

[herbdool]

I was looking first at backdrop_add_css https://docs.backdropcms.org/api/backdrop/core%21includes%21common.inc/function/backdrop_add_css/1 because I need to be able to add "integrity" and "crossorigin" attributes (maybe others).

So maybe we had changed backdrop_add_js already. That rings a bell. I suppose I can update this issue.

[herbdool]

Aha! Here it is for backdrop_add_css #2877. And I helped with the PR originally made by @quicksketch on the d.org issue. But later on in the issue Pol adds support for CSS as well. I think that threw me off.

[herbdool]

@indigoxela I've got a PR here if you'd like to look. The test could be more robust. I mentioned it could be more like JavaScriptTestCase::testAttributes.

I've tested it manually with an external css file and it works well.

[quicksketch]

I posted a question to the PR about what $group['attributes'] does: https://github.com/backdrop/backdrop/pull/3555/files#r588823603

[indigoxela]

@herbdool To be honest, I've never had a use case for additional attributes on css files. Can you give me some instructions, how to test that? Which attributes are valid (and useful) for <link> tags?

[herbdool]

@indigoxela you could try to build a link like on this page https://forkaweso.me/Fork-Awesome/get-started

<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/fork-awesome@1.1.7/css/fork-awesome.min.css" integrity="sha256-gsmEoJAws/Kd3CjuOQzLie5Q3yshhvmo7YNtBG7aaEY=" crossorigin="anonymous">

That's how I got drawn into this issue.

[indigoxela]

@herbdool many thanks, now I got it. 👍 Subresource integrity.

This is how I tested:
In a theme I implemented hook_css_alter() and added the fork-awesome.min.css:

  $css_external = 'https://cdn.jsdelivr.net/npm/fork-awesome@1.1.7/css/fork-awesome.min.css';
  $css[$css_external] = array(
    'every_page' => TRUE,
    'type' => 'file',
    'media' => 'all',
    'preprocess' => FALSE,
    'data' => $css_external,
    'browsers' => array(),
    'attributes' => array(
      'integrity' => 'sha256-gsmEoJAws/Kd3CjuOQzLie5Q3yshhvmo7YNtBG7aaEY=',
      'crossorigin' => 'anonymous',
    ),
  );

The file gets added with the right attributes:

<link integrity="sha256-gsmEoJAws/Kd3CjuOQzLie5Q3yshhvmo7YNtBG7aaEY=" crossorigin="anonymous" rel="stylesheet" href="https://cdn.jsdelivr.net/npm/fork-awesome@1.1.7/css/fork-awesome.min.css?qpldsi" media="all" />

If the hash in the integrity attribute does not match, the browser nags into the console and refuses to load the file.

If the hash matches, the browser loads the file and the fa-xx classes work as expected.

PR works for me.

[herbdool]

Thanks for testing @indigoxela. I'm going to try expand the tests to account for grouping. See if that works.

[quicksketch]

@herbdool I don't see a big value in the support for adding attributes to an entire group. If the same attribute is needed on an entire group, an implementation of hook_css_alter() could loop over every file in a group and add the attribute to each file. Perhaps the group attributes can be removed and then the current test coverage would be acceptable?

[herbdool]

@quicksketch I've updated the PR.

[herbdool]

@quicksketch I've updated the test. It now matches the test for attributes on JS - tests a custom attribute on internal and external css.

[quicksketch]

Looks great @herbdool. Marking this RTBC. I added a @since 1.19.0 Function added. line to the docblock for backdrop_css_defaults(). Letting tests run just to be safe and then this is mergeable.

[quicksketch]

Merged backdrop/backdrop#3555 into 1.x for 1.19.0.

[quicksketch]

Reopening until this gets a change record. Nice work @herbdool and thanks @indigoxela for testing!

[jenlampton]

Change record created (but needs review, and likely needs work)
https://docs.backdropcms.org/change-records/backdrop_add_css-can-now-accept-attributes

[herbdool]

@jenlampton thanks for the change record. It needs a tweak in the after example. There needs to be an attributes array for the backdrop_add_css() example whereas currently the attributes are mixed in with other elements.

$css_external = https://use.fontawesome.com/releases/v5.2.0/css/all.css';
backdrop_add_css($css_external, array(
      'attributes' => array(
        'integrity' => 'foo',
        'crossorigin' => 'anonymous',
      ),
      'type' => 'external',
    ));

[jenlampton]

Ah thank you! Updated: https://docs.backdropcms.org/change-records/backdrop_add_css-can-now-accept-attributes

[drupol]

Nice to see this in!