Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#23128 closed defect (bug) (fixed)

Twenty Twelve: "Post Author" included as hCard names in comments

Reported by: BrazenlyGeek Owned by: lancewillett
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.5
Component: Bundled Theme Keywords: has-patch needs-testing
Focuses: Cc:


When Twenty Twelve output comments made by the author of the post being viewed, it appends "Post Author" (wrapped in a SPAN) to the comment author's name.

The problem is that the entire thing (author's name & "Post Author") is wrapped in a CITE with hCard class "fn" applied to it.

This causes hCard parsers to consider "Post Author" to be part of the author's full name, as can be verified by parse results here (http://www.google.com/webmasters/tools/richsnippets) when running through Twenty Twelve-powered posts with comments made by the post author.

Line 289 of Twenty Twelve's functions.php is the relevant line; moving "%2$s" to outside the CITE tag is all that's needed, at least to fix the semantics problem (not sure if there's specific styling attached to the SPAN within the CITE or not).

Attachments (3)

23128.diff (1.9 KB) - added by obenland 3 years ago.
23128.3.diff (977 bytes) - added by lancewillett 3 years ago.
23128.2.diff (977 bytes) - added by lancewillett 3 years ago.

Download all attachments as: .zip

Change History (16)

#1 @SergeyBiryukov
3 years ago

  • Component changed from Themes to Bundled Theme

3 years ago

#2 @obenland
3 years ago

  • Keywords has-patch needs-testing added
  • Milestone changed from Awaiting Review to 3.6
  • Severity changed from trivial to normal

Wrapping it in another span made the most sense to me - might not be the best solution.

#3 @BrazenlyGeek
3 years ago

Could that be simplified by then removing the SPAN surrounding "post author"?

Instead of this:

SPAN CITE name /CITE SPAN post author /SPAN /SPAN

We'd then have this:

SPAN CITE name /CITE post author /SPAN

Version 0, edited 3 years ago by BrazenlyGeek (next)

#4 @obenland
3 years ago

Hm, I don't think this is really possible. We need some kind of markup to be able to style the badge.

My concerns were more about the fact that we use the tag itself (instead of a class for example) to style it and my diff introduces a second element of the same kind. We also could introduce and use a class to style it, I'm just not sure how that would affect backwards compatibility.

#5 @BrazenlyGeek
3 years ago

Can the outer SPAN itself be removed? So instead we have this:

CITE name /CITE SPAN post author /SPAN

I'm by no means a designer so I'm unsure how that affects the design aspect of it, but the idea of nesting non-semantic SPANs seems a bit much.

#6 follow-up: @nacin
3 years ago

Or we just remove the badge.

#7 @SergeyBiryukov
3 years ago

  • Summary changed from "Post Author" included as hCard names in Twenty Twelve comments to Twenty Twelve: "Post Author" included as hCard names in comments

#8 @lancewillett
3 years ago

This relates a bit to #20088 -- if we retrofit Twenty Twelve to use core markup (and not its own custom callback) this ticket will be obsolete.

#9 @lancewillett
3 years ago

The change in markup could break child theme styles (or sites using Custom CSS on WP.com) that have styled the post author text -- or hidden it, right?

#10 in reply to: ↑ 6 @lancewillett
3 years ago

Replying to nacin:

Or we just remove the badge.

We can't remove the badge since the theme has no other visual indicator of a post author's comments.

#11 @lancewillett
3 years ago

.2 patch uses an HTML-only technique to make the markup more specific without changing CSS at all. Should work perfectly for back compat, no changes needed to child theme CSS.

3 years ago

#12 @obenland
3 years ago

Wow, nice patch!

3 years ago

#13 @lancewillett
3 years ago

  • Owner set to lancewillett
  • Resolution set to fixed
  • Status changed from new to closed

In 23716:

Twenty Twelve: update post author comment markup and styles, improving hCard data parsing and simplifying styles. Fixes #23128.

Note: See TracTickets for help on using tickets.