Accessibility problem with headings in issue queue

Just removing empty <h3 class="comment-title"></h3> is fine too.

I don't know where to find the relevant code. I grepped around but didn't find it.

In that case, based on modules/comment/comment.tpl.php

Maybe we should just use:

  <?php if ($title): ?>
    <h3<?php print $title_attributes; ?>><?php print $title ?></h3>
    <?php print render($title_suffix); ?>
  <?php endif; ?>

I will follow-up --> Including patches #2170251: Comment headings can spit out empty heading tags

Patched and tested on http://search_api-drupal.redesign.devdrupal.org/node/2164937

I forgot that the <h3> needs to be wrapped around the permalink. Thanks for the reminder Matt!

With better pronunciation for screen reader users.

I checked that the new HTML

<div class="comment clearfix">
<h3>
<a class="permalink" rel="bookmark" href="/comment/8291535#comment-8291535">#3</a>
</h3>
</div>

is still styled by the same CSS:

.comment .permalink {
  font-size: 1.1538em; 
  font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
  font-weight: bold;
  line-height: 1.6875em;
}

I don't understand what isn't the same size. Just let me know and I'll write it up.

I applied the bluecheese patch only to http://search_api-drupal.redesign.devdrupal.org/node/2159813

Great to hear that t() is being added. I think I was thinking Bluecheese rather than the project issue which of course would need to be multilingual.

This patch is just the addition of i18n around the "Comment #". I thought the context with the span may be useful so didn't just do a simple t('Comment').

This looks good to me. This patch addresses the sass files (which is great). Also useful to see this change in the CSS in sites/all/themes/bluecheese/css/styles.css

+.comment h3.permalink-wrapper {
+  margin: 0;
+}
 .comment .permalink {
-  font-size: 1.1538em;
+  font-size: 0.9375em;
   font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
   font-weight: bold;
   line-height: 1.6875em;

I don't know which environment to test this in.

@drumm would you want this tested somewhere else before marking it RTBC?

And they look good there.

I think so.

Sorry, Matt. I saw your not after I'd written the patch.

That error is mine.. I should just comment out the code.. Will do that later today..

I've made enough little changes to it I'm getting to the stage where I want to do.

bzr reset --hard

I just don't know if that exists or what that would do to others working on the code....

Thanks for committing the project_issue portion.

Good to keep the theme customizations to a minimum.

Yes. i started that issue on D8 back at the beginning of January. It can be hard sometimes to get traction on various Core issues. Resources are scarce as you know.

So you'd prefer to put it all into the preprocess function with the H3's, and then simply do something like <?php print $permalink ?>

Agreed about keeping the code together where possible. I've just poked around as you know and it's easy to get lost.

EDIT: That should be <?php print $permalink_title; ?>

Both drupalorg_project_preprocess_node() & drupalorg_project_preprocess_comment_wrapper() are mucking with the comments in drupalorg_project.module. But in the latter. I think that this would be the best place to put a $permalink_title:

function drupalorg_project_preprocess_comment_wrapper(&$variables) {
  if (project_issue_node_type_is_issue($variables['node']->type) && !empty($variables['content']['comment_form'])) {
    $update_link = 'node/' . $variables['content']['comment_form']['#node']->nid . '/edit';
    $variables['comment_form_title'] = t('Add new comment') . ' | ' . l(t('Update this issue'), $update_link);
    $variables['permalink_title'] = (!empty($variables['title'])) ? '<h3 class="permalink-wrapper">' . $variables['permalink'] . '</h3>' : '';
  } 
}

t('Add new comment') . ' | ' . l(t('Update this issue'), $update_link);

Can you just get rid of this empty heading here:

<h3 class="comment-title"></h3>

Otherwise it looks ok:

  <div class="submitted">
    <h3 class="permalink-wrapper"><a rel="bookmark" class="permalink active" href="/node/2240063#comment-8687911"><span class="element-invisible">Comment </span>#4</a></h3>    Posted by <a class="username" title="View user profile." href="/user/1853874">formatC'vt</a> on <time datetime="1397646628" pubdate="">April 16, 2014 at 11:10am</time>      </div>

Yes, but we're not sure how to replicate this problem in D8 #2170251-20: Comment headings can spit out empty heading tags

Plus, it's a pretty trivial fix in the template. It could just be removing the heading in bluecheese.

In /tpl/comment.tpl.php we can just delete this line, right:

<h3<?php print $title_attributes; ?>><?php print $title ?></h3>

With all the changes with Twig, I really don't know that we need to wait for this to be backported from D8 to D7 in order to fix this accessibility problem that is affecting folks on drupal.org right now (and for the last 9 months).

Looks good.

The empty headings is still a problem and really don't think it's something that will take the year or so that it may take to get through to a D7 Core release.