Problem/Motivation
A client defined that as a requirement.
Not quite sure where to put this. I found this old issue in views #988474: Table header ordering links should have rel="nofollow" in the <a> tags which was closed.
As mentioned there, drupal.org has that as a custom robots.txt rule:
# Extras on drupal.org
# no access for table sorting paths or any paths that have parameters.
Disallow: /*?sort*
Disallow: /*&sort*
I guess that's a bit too wide to have by default., no way to know whether people use sort=something and want to have it indexed.
Proposed resolution
Add that in tablesort_header()
Remaining tasks
User interface changes
API changes
Data model changes
Comments
Comment #2
yaremchuk commentedLet me take that issue, will work on them on DrupalEurope.
Comment #3
yaremchuk commentedit fixed the issue in native drupal tables and in views tables
Comment #4
yaremchuk commentedI just realized that classy and stable overwrite views table templates...
Comment #5
yaremchuk commentedAlso, I have added the same fix in Classy and Stable themes.
Comment #7
berdirAlso found #1758354: Optionally add Robot Meta Tags (nofollow, noindex) to sort links, and Views subpages, with quote from @dawehner:
> As views will land into core such crappy seo stuff shouldn't be there.
> As far as i know you can even fix that with a proper written robots.txt.
Problem is, it doesn't look like the robots.txt approach is actually working, somehow google gets pretty confused about that, we have thousands of warnings in google search console with that approach where google apparently indexed those links but then later complains that they are not allowed. So it seems that adding a nofollow upfront would make sense and avoid those index requests in the first place. I don't think that's "crappy SEO stuff" :)
Comment #8
mbovan commentedThe patch #5 does not apply anymore.
I rerolled and added the missing test coverage.
Comment #9
borisson_I agree with @Berdir, this is a good thing to do. I also think that having some best practices in the core for SEO is not a bad thing at all.
For nice search robots, this will reduce server-load as well, making all drupal sites more eco-friendly ;).
It has testcoverage and the patch is really small. It looks like this might be a BC break though but I'm setting this to RTBC anyway to surface this to core maintainers who have more knowledge about BC.
Comment #10
catchThis definitely needs a change record to note the change in the templates.
I think the change itself is fine to commit to a minor release, but marking for frontend review to ensure the stable/classy changes are appropriate - I think it might depend if we consider this a feature request or a bugfix. CNW for the change record.
Comment #11
lauriiiYeah, could definitely use a change record for the markup change, but other than that this change should be fine to be applied to Stable and Classy in a minor release.
Comment #13
mbovan commentedI created a draft change record at https://www.drupal.org/node/3092185.
Additionally, I have checked Twig templates added by Claro theme and it has no implementation of
views-view-table.html.twigtemplate. That said, #8 should still be commit-ready patch.Comment #14
mbovan commentedSince #8 passed 8.9.x tests, the change record was created and the issue was reviewed by the frontend framework manager in #11, I am updating the status to Reviewed & tested by the community as previously set in #9
Comment #15
alexpottCommitted and pushed dc0b137553 to 9.0.x and eb89cf435a to 8.9.x. Thanks!