Remove unnecessary 'rdf-meta' class from HTML output [#653916]

Spin off from #493030-196: RDF #1: core RDF module

The RDF module introduces some theme related use-cases that as far as I know are new to this version of Drupal, so I can't find best practice information for. For example, we add empty spans to the HTML body. Should these empty spans have a class (e.g., 'rdf-meta')? Normally, when we add a class to a tag, it's because there's a potential use-case for a theme developer wanting to style it. But here we have something that's not intended for any styling. I argued for the class, because as a programmer, I imagined that it serves as useful in-line documentation. A theme developer viewing the HTML source can see that class and then grep Drupal code for it, and see the line of code that adds it, and have a better understanding of what purpose the tag serves. Also, not being myself a CSS expert, I thought maybe there are situations where you would need to explicitly target it with a 'display:none' rule.

scor argues that you would never need to target an empty span with a 'display:none' rule. I sent John Albin a PM asking him to comment, but he hasn't responded (he's probably busy with more pressing issues). I asked another CSS expert about this, and she agrees with scor, that you should never need a class on an empty span that is never intended to be visible. She also says that it would confuse theme developers more than help them to have the class. According to her, CSS developers assume that classes are there to be used (at least potentially) and seeing a class would make them want to know what they're expected to do with it, which sends the wrong message, because the message we really want to send is: even though this markup is here in the HTML body, it serves no display purpose, so pay no attention to it from a styling standpoint.

I don't know what we can do within the HTML source to convey that message, so if you have ideas, please comment on this issue. In the meantime, here's a patch that removes the class.

Comment	File	Size	Author
	drupal.rdf-meta-remove-class.patch	996 bytes	effulgentsia

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

sun

German

Karlsruhe

CreditAttribution: sun commented 8 December 2009 at 00:50

I'm not sure. There can be egde-case scenarios.

.comment span {
  display: block;
  height: 10px;
  background-color: #ccc;
}

Comment #2

scor CreditAttribution: scor commented 8 December 2009 at 12:12

effulgentsia summarizes the discussion we had in the original thread. I'm still unsure about this... subscribe

Comment #3

mlncn CreditAttribution: mlncn commented 13 December 2009 at 17:13

Issue tags:

+RDF, +markup

Tagging and subscribing. I would RTBC this but the inline commenting effect of having the class is persuasive for me, so people have some hope of finding out why they have empty spans. Where can we put real documentation a themer might hope to find?

Comment #4

effulgentsia CreditAttribution: effulgentsia commented 12 March 2010 at 17:37

Component:	rdf.module	» markup
Issue tags:	-markup

Moving to "markup" component. Although I posted the patch to remove this class, I'm just not sure if it should be or not. I'm perfectly fine with this issue either being RTBC'd or set to "by design". Just wanted to make sure it had a chance to be reviewed.

Comment #5

Anonymous (not verified) CreditAttribution: Anonymous commented 12 March 2010 at 19:30

I would argue for "by design". I'm not a CSS expert, but I have done a good amount of theming and having this extra class wouldn't confuse me. I may be different from others, but I don't see a class and think "I need to do something with that". Instead, I look at my id and class options and see what elements I want to target with CSS. Since I would see no functional need to target an empty span (unless I actually *did* want to target that span), I wouldn't think that I need to apply any CSS rules to the class.

However, if I was looking at the source code and saw empty spans scattered around, I might think I was doing something wrong. I think the rdf-meta tag would be helpful in that situation, to give a clue as to what this tag is there for.

So yeah, I vote we keep the class in.