Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#56444 closed enhancement (fixed)

Remove rel="nofollow" from internal links in comments

Reported by: benish74's profile benish74 Owned by: jorbin's profile jorbin
Milestone: 6.2 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch has-unit-tests
Focuses: Cc:

Description (last modified by sabernhardt)

Hello,
I hope it's the right place to post this suggestion,
I think it would be useful not only for me, but for many WordPress users.
The suggestion is to remove rel="nofollow", only from internal links, in the comment section of WordPress.

All the best,
Ben

Attachments (1)

56444.diff (462 bytes) - added by sabernhardt 3 years ago.
remove nofollow but keep ugc for internal links

Download all attachments as: .zip

Change History (24)

@sabernhardt
3 years ago

remove nofollow but keep ugc for internal links

#1 @sabernhardt
3 years ago

  • Description modified (diff)
  • Focuses coding-standards removed
  • Keywords has-patch added

Hi and thanks for the ticket!

At first, I thought this was related to #16881, but that is about the reply links.

Altering the rel for links within the comment content is possible already with the make_clickable_rel filter, though I attached a patch to change the default.

#2 @benish74
3 years ago

Thank you so much for taking my suggestion in consideration.

Ben

Last edited 2 years ago by audrasjb (previous) (diff)

#3 @samiamnot
3 years ago

@sabernhardt ,
What about relative links that are by nature internal links? The patch does not seem to address that.

#4 @sabernhardt
3 years ago

  • Keywords needs-patch needs-unit-tests added; has-patch removed

The patch was not thorough; it only addresses URLs that are made clickable. For links that are already in an a element, a patch would need to edit wp_rel_ugc() as well.

#5 @desrosj
2 years ago

  • Milestone changed from Awaiting Review to 6.2

I came across this debugging some SEO issues being flagged on a site today. This seems to make sense, and it would be nice to get this into 6.2.

#6 @desrosj
2 years ago

This also looks like a duplicate of item number 3 in #53290. However, that ticket does not reference make_clickable() (only wp_rel_callback()), so I'm leaving this one open for now.

This ticket was mentioned in PR #3825 on WordPress/wordpress-develop by thomasplevy.


2 years ago
#7

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

#8 @thomasplevy
2 years ago

  • Keywords needs-patch needs-unit-tests added; has-patch has-unit-tests removed

I opened a PR (https://github.com/WordPress/wordpress-develop/pull/3825) for review with an alternate patch to address this issue.

It also, I think, addressed the issues raised in #53290

I think it addresses all the concerns and I think I have it well covered with tests. I did add a new "helper" function. I was trying my best to repeat as little code as possible across the modified functions and creating a new helper seemed the best way to do this, but I'm not sure I've implemented this in the best way possible: does it belong in this file? Should it be "private" (as I've made it) with a leading underscore? Should it not exist at all? Is it well named? Etc...

Looking for feedback if this patch would be a good way to address these problems. If it's good to go I'll convert to a patch and attach here.

Thanks!

#9 @sabernhardt
2 years ago

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

This ticket was mentioned in Slack in #core by costdev. View the logs.


2 years ago

#11 @desrosj
2 years ago

I added some comments/questions in a PR review. In general, I like the direction.

I'm not sure that this solves the issue within get_comment_author_link() noted in #53290, but the other instances seem to be addressed.

thomasplevy commented on PR #3825:


2 years ago
#12

@desrosj I've updated based on your feedback and also covered the missed point 1 on https://core.trac.wordpress.org/ticket/53290 regarding get_comment_author_link().

#13 @thomasplevy
2 years ago

@desrosj updated per your feedback, thanks!

thomasplevy commented on PR #3825:


2 years ago
#14

Running tests locally using the in-built env scripts and forcing php 8.1 via LOCAL_PHP="8.1-fpm" npm run env:start results in a passing test suite. I didn't try on 8.2. I haven't used the develop test suites a ton so I'm trying to figure out if these are expected to fail or if new code broke them. It seems like new code broke them but I can't recreate that locally to debug further.

#15 @desrosj
2 years ago

  • Owner set to desrosj
  • Status changed from new to reviewing

thomasplevy commented on PR #3825:


2 years ago
#16

I think this comment got buried: https://github.com/WordPress/wordpress-develop/pull/3825#issuecomment-1411162430

I went ahead and dropped the PHP 8.1+ conditions in the related tests. I can rollback the commit if that's problematic.

thomasplevy commented on PR #3825:


2 years ago
#17

@aaronjorbin I spoke briefly with @desrosj about this and he said he'd probably be looking at this tomorrow but if you want to push back on any of my above comments let me know and I'll make some changes tomorrow.

#18 @jorbin
2 years ago

  • Owner changed from desrosj to jorbin

This ticket was mentioned in Slack in #core by jorbin. View the logs.


2 years ago

#20 @jorbin
2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 55289:

Comments: Improve rel attribute usage in comments.

Internal links should be followed and it should be easier to modify other rel attributes on comments. This adds a helper function for determining if a URL is internal and also adds some new filters to make it easy to modify rel attributes in comments.

Props thomasplevy, desrosj, sabernhardt, benish74, samiamnot, galbaras, jorbin.

Fixes #53290, #56444.

thomasplevy commented on PR #3825:


2 years ago
#21

@aaronjorbin I may have deleted and closed too quickly, it looked like you were done.

If you want me to reopen let me know

Sorry

@jorbin commented on PR #3825:


2 years ago
#22

@thomasplevy It's all good, you closed it right on time. Thank you for your work on this!

#23 @SergeyBiryukov
2 years ago

In 55495:

Formatting: Restore consistent quotes in _make_web_ftp_clickable_cb().

After the introduction of _make_clickable_rel_attr() in an earlier commit, the function ended up returning link markup with a mix of single and double quotes.

This commit ensures that _make_web_ftp_clickable_cb() always returns double quotes, restoring consistency with other similar callback functions used by make_clickable():

  • _make_url_clickable_cb()
  • _make_email_clickable_cb()

Follow-up to [55289].

See #53290, #56444.

Note: See TracTickets for help on using tickets.