I think we're almost there in terms of core theme layer additions needed for RDF. #569362: Document $attributes, $title_attributes, and $content_attributes template variables and #565792: Refactor theme_username so that RDF patch can be accepted are both in HEAD now, yay! But see http://drupal.org/node/493030#comment-2031884. Seems like we need something for timestamps. Note, date fields will be handled with field.tpl.php, but timestamps aren't yet handled properly anywhere.

Should we introduce a new theme hook (theme_timestamp())? And change some/all of the places that call format_date() (for example, template_preprocess_node()) to call theme('timestamp') instead?

Before proceeding with a patch submission, anyone care to share thoughts on whether this is a good idea, and what options / functionality this theme hook would need to support?

Comments

effulgentsia’s picture

effulgentsia commented

To summarize the issue here, especially for anyone who hasn't followed the other theme layer issues related to RDF, we are trying to make sure that all places where attributes need to be added to a markup tag are done so via the theme layer, so a themer has full control over all markup. We don't want to have the RDF module creating span tags in a way that's hidden from a themer.

catch’s picture

catch
he/him
Primary language English
commented

We just removed theme_node_submitted() from core, which saved us a duplicate format_date() on node listings, so adding a theme() wrapper around the date itself probably wouldn't be too bad in terms of performance trade-offs overall, but this should probably still get benchmarks/profiling.

scor’s picture

scor commented

the very first patch dealing with date #352204: RDFa: author and date on node and comment was written when theme_node_submitted() and theme_comment_submitted() were still in core. I'm not sure how I would do it now...

fago’s picture

fago
Primary language German
Location Vienna
commented

But do we really want to theme it? We just want to add the the wrapping span in a way, the themer can override it. Having to override a theme function just to remove those attributes feels like being a bit to complex for me, ideally this could be done directly in the template.

So what about enhancing theme_markup() to generate a wrapping span if some #attributes are set for an element? That way we could simply do

 $variables['date'] = array('#markup' => format_date(..));

in the node preprocess. Rdf module can add in #attributes later on. Then in the theme just use render($date); If the attributes are unwanted, one could just unset/alter them before rendering. Also we could apply this pattern for all other variables too, that miss a way to add in attributes.

fago’s picture

fago
Primary language German
Location Vienna
commented

Just implemented the solution described in #4 in the patch of http://drupal.org/node/493030#comment-2033900. Comments?

catch’s picture

catch
he/him
Primary language English
commented

#4 sounds better than a new theme function to me.

effulgentsia’s picture

effulgentsia commented
Status: Active » Closed (won't fix)

The specific RDF use-case was solved in #493030-157: RDF #1: core RDF module with the introduction of theme_rdf_template_variable_wrapper() and the $variables['rdf_template_variable_attributes_array'] template preprocess variable, so that RDF can wrap any template variable that exists as a string rather than a render array. There's an issue, #633642: Rename $variables['rdf_template_variable_attributes_array'] to something sane, to rename the variable and theme hook to something shorter.

I hope that for Drupal 8, more template string variables are converted to render arrays, in which case, something along the lines of #4 would be great, but for Drupal 8, we may refine everything having to do with rendering, so it'll make sense to open issues like this down the road, once we know what that rendering model solves and what unsolved use-cases remain.

effulgentsia’s picture

effulgentsia commented
Priority: Critical » Normal
Issue tags: -RDF, -RDFa, -API clean-up

Since the RDF use-case was solved, introducing theme_timestamp() certainly isn't critical, and the issue tags no longer apply.