Document $attributes, $title_attributes, and $content_attributes template variables

No way we can just replace $classes with $attributes rather than adding it?

That's a good enough argument for me, and in general this patch looks great, so no objections. I don't have time to do an in-depth review tonight though, so leaving to others to rtbc.

Alex: Awesome! Thanks for tackling this, and thanks for seeking our input during the sprint. I'm very happy with the implementation we discussed.

I believe webchick's biggest concern with this patch will be whether some themers will be unhappy with the added complexity of the new variables: $attributes, $title_attributes, and $item_attributes (on field.tpl.php only). So, themers: please add your thoughts about this.

This is a matter of trade-offs. The only other solution -- wrapping this information in spans or other markup -- is completely undesirable, so our next-best option is to add $attribute variables. This, however, isn't really a big deal. If themers are working on sites that require RDF/RDFa data, they need to know how to properly support it. RDF/RDFa data is, after all, a matter of markup.

Themers: please also comment on what I said in comment #3: that you don't want 'id' and 'class' in the attributes variable by default, so that you can deal with those in your tpl.php file independently, but that for those of you that want to merge them in, you want the flexibility to do so in your template.php's preprocess function(s).

The type of themers who would want to combine class, ID, and other attribute data into a single variable are savvy enough to know how to do so in preprocess/process functions. By adding elements' class and ID attributes separately, we provide an easy method for non-savvy themers to add these values directly to markup. This is essential to retaining a good mix of savvy and non-savvy theme accessibility.

I reviewed this at the sprint and +1 it. Since Todd is happy as well, lets proceed. This approach is consistent with our granular themeing in D7.

I reviewed this patch and it looks good to me. I agree with the careful analysis in this issue -- it is the best solution. I'll leave this RTBC for a bit longer in case more people want to chime in.

I would like to know more about the changes that this will make to the output markup for a page. Is there an example site that I can visit where this patch is implemented? What attributes are being added to elements here to store the RDF data?

@Everett Zufelt: with the current patch, a node markup would look like this (note the new attributes @about, @typeof and @property):

    <div id="node-1" class="node node-article node-promoted node-teaser clearfix" about="node/1" typeof="sioc:Item foaf:Document">
      <h2 property="dc:title"><a href="node/1">title of the post</a></h2>
      <div class="meta">
        <span class="submitted">
          Submitted by <a href="user/1" title="View user profile." class="username">admin</a> on Thu, 09/10/2009 - 07:56
        </span>
      </div>
      <div class="content">
        <div class="field field-name-body field-type-text-with-summary field-label-hidden clearfix">
          <div class="field-items">
            <div class="field-item even" property="content:encoded">
              <p>Content of the post...</p>
            </div>
          </div>
        </div>
      </div>
    </div>

The patch in #1 is functional. I would like to suggest an extra measure to ensure we don't allow invalid HTML to be generated. Currently if one writes the following in a preprocess function:

  $variables['attributes_array']['class'][] = 'customclass';

it will display 2 class attributes in the same tag. The patch attached moves these classes in the ['classes_array'] which are inserted in the tpl file separately (reproducing the same behavior as pwolanin's block module patch).

I see your reasoning, which is to let the themers ensure they produce valid HTML. It also saves us some extra processing in the theme layer. setting back to RTBC with the initial patch.

Committed to CVS HEAD. Thanks!

All these new 'attributes' template variables are being used solely for RDFa. So let's make it clear in the docs that this is all they ARE being used for, and that this is all they SHOULD be used for. I don't see why we should suggest that users overload these variables with non-RDFa XHTML attributes (eg class and id attrs).

Updated patch, which is the same as that in #20, but which:

1. replaces all wording "String of XHTML attributes" to "String of RDFa attributes" (this is still technically just as accurate, only more specific, since RDFa is a representation of RDF data in valid XHTML form)
2. removes the lines "By default, this variable does not include the id and class attributes, so that the template can separately control how these are printed. At its discretion, the theme may use a preprocess function to merge the id and/or class into this variable, in which case the template should not also print them separately." We don't need this text if we make it clear that only RDFa attributes can/should be in this variable.

Personally, I'd also advocate renaming all the variable names to $[foo_]rdfa_attributes[_array], but I'll just leave that as a suggestion for now.

#608036-12: Add content_attributes variable for tpl files, so RDF can be implemented better. introduced $attributes in user-profile.tpl.php and I think some other user*.tpl.php files are missing from that patch. I would suggest to grep HEAD to make sure all tpl containing these variables have been covered.

@effulgentsia:

Re: use of $attributes for other non-RDFa attributes (e.g. class and id). Comment #7 (which you pointed to) doesn't give a particularly strong argument - I'd summarise it as: "advanced themers _might_ prefer to inject data into $attributes through a preprocess function; and non-advanced themers almost certainly _won't_, they'll prefer instead to put other attributes directly in their templates." Templates should be accessible to people with HTML/CSS skills but not PHP skills. Being able to output class and id attributes directly in your template is designer-friendlier than having to write a preprocess function to do the same thing. IMHO, encouraging $attributes to contain _only_ RDFa data is more accessible to such people (and those are the people we want as Drupal themers, right?). So, I do ask once again that you accept my amended patch.

Re: not documenting every variable of every template in detailed comment docs. Yes, it's a lot of overhead to maintain such docs - more than just putting a "see this handbook page for more info" line. But it's also extremely useful and convenient for themers. Once again, we want to make templating as easy as possible for non-coders. Is it so much to ask, that when a non-coder (i.e. someone whose PHP skills don't go far beyond print'ing) opens up a .tpl.php file, that he/she can quickly see every variable that's available for printing in that template? I vote for maintaining the (growing) amount of duplication in template comment code, as the price of a superior themer experience. (Anyway, this is off-topic, so I'll stop now).

#21: drupal-doc-theme-attributes-569362-21.patch queued for re-testing.

There are many things that these variables can be used for, including RDF, WAI-ARIA, adding a class to the title of a a block generated in code (w/o a template).

Working on this. This issue needs backport to D7, as the $content_attributes is not documented there neither (See https://www.drupal.org/node/2332087 )

Updating this to D8 first

As seems that there is no consensus in the discussion about saying RDFa or other thing, I just done a quick reroll for D7 and created a patch with the content that was missing for Drupal 8.

Changing status for review and unassigning

Thanks for reviving this issue!

There are a few problems with this patch, I think:

a) block.html.twig

- * - attributes: HTML attributes for the containing element.
+ * - attributes: String of RDFa attributes populated by modules, intended to

Ummm... I don't think that all of the attributes on a block are RDFa attributes, are they, by far?

b) block.html.twig

+ * - attributes: String of RDFa attributes populated by modules, intended to
+ *   be added to the main container tag of this template. This is either an
+ *   empty string or a string containing a leading space, so it should be
+ *   printed without a leading space before what precedes it.

Is this about it being a string etc. and the leading space really accurate? It seems like it's an array or object when it comes in, based on how the template uses it:

<div{{ attributes.addClass(classes) }}>

c) block.html.twig

+ * - $title_attributes: Same as $attributes, except applied to the main title
+ *   tag that appears in the template.
+ * - $content_attributes: Same as $attributes, except applied to the main
+ *   content tag that appears in the template.

These should not have $ on them. There are no $ on these variables in Twig.

d) content_attributes is not actually printed in the template block.html.twig that I can see?

e) comment.html.twig

+ * - title_attributes: Same as attributes, except applied to the main title
+ *   tag that appears in the template.
 

There is no documentation in this template for attributes, just attributes.class, so saying "Same as attributes" doesn't really help?

f) All of these templates have copies of the documentation in the Bartik overrides of these templates, so the patch needs to fix those too. I personally think that it would be better if Bartik would not copy documentation when it overrides a template because of the need to maintain the docs in two places, but ... that's a separate discussion.

Ok, I think I fixed all the comments from the review on #38. Here is a new patch. I also updated all the affected bartik templates. Once that patch is RTBC, I will update the D7 patch, if It's necessary.

Thanks, this version looks good to me!

Back to RTBC. I would just commit it but there were many people participating; would like to give them one more chance to set back to needs work if necessary.

It's been a day, but not sure what sort of time you wanted. Assigning to jhodgdon. :)

Committed to 8.0.x. Thanks! Moving to 7.x for backport of documentation, as much as is applicable anyway.

The patch in #39 only has block, comment, and node templates in it. No field.html.twig. The patch did not change the content_attributes section of the comment templates.

So... the patch as supplied was committed... do we need a new patch?

This was assigned to me for committing...

Here's the patch for D7.

Patch looks fine, thanks! To be fixed on commit:

1. +++ b/modules/field/theme/field.tpl.php
   @@ -23,7 +25,10 @@
   - *
   

   missing newline

2. +++ b/themes/garland/comment.tpl.php
   @@ -1,5 +1,3 @@
   -<?php
   -?>
   
   +++ b/themes/garland/node.tpl.php
   @@ -1,5 +1,3 @@
   -<?php
   -?>
   

   Unrelated changes :)

Hi!

Here's the updated patch.

+ * - $attributes: array of HTML attributes populated by modules, intended to
+ *   be added to the main container tag of this template.

This variable is a string, not an array.

Also the patch seems to be missing some of the changes that were there before, including the one that caused #2332087: undocumented $content_attributes in node.tpl.php to be marked as a duplicate of this issue. The earlier Drupal 7 patch from David Hernández in #33 looks like it had more of those changes. (I don't think it's the end of the world if we commit something that isn't 100% comprehensive, but we should at least try to keep all the templates somewhat consistent.)

Going to work on that, patch coming in the following hours.

Here's the updated patch.