Great that there's a link to report spam:
<a href="/node/add/project-issue/webmasters?component=Spam&categories=task&title=Spam%20Report&body=Reporting%20the%20following%20comment%20as%20spam%3A%20https%3A//git7site.devdrupal.org/node/1339130%23comment-7835723">report spam</a>
I did wonder if this should be capitalized, but there's an accessibility problem with this.
It should be something more like:
<a href="/node/add/project-issue/webmasters?component=Spam&categories=task&title=Spam%20Report&body=Reporting%20the%20following%20comment%20as%20spam%3A%20https%3A//git7site.devdrupal.org/node/1339130%23comment-7835723">report spam<span class="element-invisible"> on user USERNAME</a></span>
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | Screen Shot 2014-01-22 at 11.29.52 AM.png | 162.5 KB | mgifford |
| #18 | report-span-accessibility-2095983-18.patch | 1001 bytes | mgifford |
| #14 | report-span-accessibility-2095983-13.patch | 792 bytes | mgifford |
| #11 | report-span-accessibility-2095983-11.patch | 913 bytes | mgifford |
| #7 | report-span-accessibility-2095983-7.patch | 703 bytes | mgifford |
Comments
Comment #1
tvn commentedComment #2
jaypanThe HTML is invalid, as the span and anchor tags overlap:
<a><span></a></span>Before I fix this, can I clarify that it should be:
<a href="/node/add/project-issue/webmasters?component=Spam&categories=task&title=Spam%20Report&body=Reporting%20the%20following%20comment%20as%20spam%3A%20https%3A//git7site.devdrupal.org/node/1339130%23comment-7835723">report spam<span class="element-invisible"> on user USERNAME</span></a>Thank you.
Comment #3
mgiffordYup. That looks good. Cut/paste error, thanks for catching it.
Comment #4
mgiffordNote this is still an error and a violation of WCAG 2.0 AA guidelines.
Comment #5
jaypanSorry, I'm releasing three sites in November, a little bogged down now. I don't have time to figure out how to work in my development environment just yet. After these sites are live (they all should be by the end of the month) I'll be taking care of this.
Comment #6
mgiffordThanks for the update. Good luck with the launches.
Comment #7
mgiffordI think this is it but I'm having trouble logging in.
It doesn't address drupalorg_node_view_alter(), but I can't find the links to the "Report Spam" on notes either.
Also I note that there's some inconsistency in case in that the comments are all "report spam".
Comment #8
drummCommitted, http://drupalcode.org/project/drupalorg.git/commit/795ca01. This will go out later today when drupalorg module is merged and deployed.
I had to add
'html' => TRUEso the markup was rendered properly.Setting back to active for
drupalorg_node_view_alter().Comment #9
mgiffordThanks for adding in the HTML setting! I thought I'd got that in but apparently not.
Comment #10
drummThis patch caused another problem, #2175545: Including the username in the hidden part (class="element-invisible") of the "report spam" links is not a good idea.. I'll be rolling it back soon. Having the wrong username is worse than no username.
Comment #11
mgiffordSorry to hear that. Right.. So the $user is just for a comparison:
if ($build['#comment']->uid != $user->uid) {How did we miss that when looking at the source.. Ah, yes.. I wasn't logged in at the time... It's obvious on d.o...
Ok, this is tested now to show the appropriate comment username.
Comment #12
drummA new user_load() should not be added to this already heavy page (unless really needed). In this case, it isn't needed, comments come with a
->nameproperty.Comment #13
drummDeployed the revert of #7, http://drupalcode.org/project/drupalorg.git/commit/24398f5
Comment #14
mgiffordThat's better.. Thanks!
Comment #15
jaypanThanks for taking care of this - apologies I was so slow.
Comment #16
mgifford@drumm Agree with your decision to revert. But now that we've got a patch what else do you need?
Do they need to be re-combined? Marked RTBC again?
@Jaypan - it happens. Will be good to see this issue resolved.
Comment #17
drummCombining so this patch applies against a checkout of this module's Git repo would be ideal.
Comment #18
mgiffordI applied it here http://spam-drupal.redesign.devdrupal.org/node/2170251
I think that should give you what you need.
Comment #19
jaypanComment #20
drummCommitted.
Comment #21
drummDeployed.
Comment #22
mgiffordThanks @drumm! Looks good. Sorry for the earlier mess-up on this issue.