Text in new topic and new post links does not describe its purpose

Thanks for catching and posting this.

If I understand correctly, you are saying that since each link has the anchor text "1 new" that it is ambiguous. I would agree in part.

I would argue that based on the location in the table that the link purpose is programmatically determinable, but we should make sure that the first column is marked as a header, using whichever technique is best supported by AT.

I also question whether the links aren't equally ambiguous to all, but there is not reason that this should prevent us from improving accessibility where we can.

Setting to 7.x-dev, as this is a markup change that likely can be patched into a 7.x release.

Tagging

@Daniel

Thanks for the additional resources. One problem is that AT (screen-readers) don't actually honour the web standards contract. So, they poorly support table header information. This doesn't mean that we should not mark up the first column as a header, we're responsible for our side of the contract whether the AT vendors hold up their end of the deal or not.

So, I'm just pondering what the best method is for us to mark up that column, and then thinking that we need to be consistent with tables across Core, which will be a D8 issue for sure.

See: http://tink.co.uk/2010/08/screen-reader-support-for-html-tables/

I'm interested in testing a patch for this when it comes out.

It sounds like we're all agreed with option #2: "Making the first cell in the row a header row with a scope='row'". I support that approach. +1.

So we need a patch. I'm also unsure if we should be applying this to D8 & then back-porting it at this stage.

I believe we would apply this to D8 and then decide whether to back-port to D7. This patch would introduce a new t() string, something like this: 'in %forum_title'. We don't have that string translated already so we would need further clarification to decide whether this patch would be accepted into D7.

Too bad we can't just translate 'in'...

#9: Well, the concept of "in" is going to be vastly different in different languages. In some languages the equivalent word would actually have to come after the forum title, for example. In some languages there isn't an equivalent word. Etc. I think the t() actually needs to go around the whole thing: t('%count new posts in forum %title') or such. So I don't think it can be backported. Maybe we could provide a workaround for D7.

The technique in #2 in the original post sounds interesting, but I don't think it's quite as beneficial by itself as actually adding more descriptive text, which would aid all users. What about using the #title attribute for the link for the full explanation? Is that a correct solution?

Oh, re #4: is there an issue open for core table accessibility already?

@xjm Agreed with the i18n. Language constructions are difficult for sure.

As far as using #title attributes, unfortunately that doesn't work for a lot of assistive technology. We could add invisible text which provides more descriptive text, or follow the guidlines here http://webaim.org/techniques/tables/data

I don't think that there is a generic issue for core table handling, but there are lots of issues related to table accessibility in core:
http://drupal.org/project/issues/search/drupal?text=table&assigned=&subm...

It looks like two proposed solutions.

1. Modify the link text
2. make the first col a th with scope=row

We could do both.

Can someone please tell me what function themes this table, is it just theme_table()?

From documentation of theme_table()

Each cell can be either a string or an associative array with the following keys:

• "data": The string to display in the table cell.
• "header": Indicates this cell is a header.
• Any HTML attributes, such as "colspan", to apply to the table cell.

So we could either use header, or set the scope attribute in attributes.

Tagging

Everett, it's actually manually done in templates:

• forum-list.tpl.php
• forum-topic-list.tpl.php

That makes it even easier!

Attached patches for each solution.

+      <td <?php print $forum->is_container ? 'colspan="4" class="container"' : 'class="forum"'; ?> scope="row">

Curious what the colspan is about?

Everett, it was already there! :)

Working on a new patch for better link text, the old unread topic test needs to be updated because it searches for the text using xpath to determine the count.

#16: forum-1060700-scope-row.patch queued for re-testing.

#16: forum-1060700-scope-row.patch queued for re-testing.

+++ b/core/modules/forum/forum.module
@@ -1072,7 +1072,7 @@ function template_preprocess_forum_list(&$variables) {
+        $variables['forums'][$id]->new_text = format_plural($variables['forums'][$id]->new_topics, t('1 new post<span class="element-invisible"> in forum %title</span>', array('%title' => $variables['forums'][$id]->name)), t('%count new posts<span class="element-invisible"> in forum %title</span>', array('%title' => $variables['forums'][$id]->name, '%count' => $variables['forums'][$id]->new_topics)));

format_plural is a wrapper to t(), you don't need to use t inside format_plural. The variable @count is available in the second string so you don't need to pass $variables['forums'][$id]->new_topics twice.

+++ b/core/modules/forum/forum.module
@@ -1136,7 +1138,7 @@ function template_preprocess_forum_topic_list(&$variables) {
+        $variables['topics'][$id]->new_text = format_plural($topic->new_replies, t('1 new post<span class="element-invisible"> in topic %title</span>', array('%title' => $original_title)), t('%count new posts<span class="element-invisible"> in topic %title</span>', array('%title' => $original_title, '%count' => $topic->new_replies)));

same here

Powered by Dreditor.

Since this looked fairly simple, and there was no activity, I took the liberty to redo the forum-1060700-better-link-text.patch

Sorry, some mix up with versions here. Here is a new patch against latest 8-dev

Ah, sorry. That was a bit quick. Also, the test should probably be updated?

New patch attached.

Sorry for the spamming. That newbie just can't get it right, eh?

Anyway, here is a patch that does not fail the test (on my install anyway), since the test is now updated.

Patch applies nicely. No problem about the repeats with the testing. Main thing is that you pushed through and got one that worked. It looks like it should be RTBC, but I have to get my d8 system up again. I'll check back in when I do.

Nice work @eiriksm. Do we also want to add additional test assertions to check that the forum name is there in the source?

Thanks for kind feedback and testing. Here is a patch that also checks for the forum name in the test. Since I am no expert at writing tests, I just added what seemed logic to me here. So the test passes, but please feel free to advice if this is not entirely correct.

Thanks eiriksm, looks good to me.
Can you post a tests only version to verify that the new assertion fails without the new logic?

Sure. This is the test only. Fails for me, should fail here

As expected. Changing status back

Looks good to me

Makes sense, committed/pushed to 8.x.

The patch from this issue introduced an undefined variable error, as talked about here:
https://drupal.org/node/1796292
This new issue also addresses the problem.