Make WordPress Core

Opened 12 years ago

Last modified 21 months ago

#22889 accepted enhancement

Reconsider no-JS ?replytocom= links

Reported by: markjaquith's profile markjaquith Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch dev-feedback
Focuses: Cc:

Description

We have a no-JS fallback for comment replies. Normally JS moves the comment form around. For people with JavaScript disabled, they follow the ?replytocom={123} link. This results in a lot of extra crawling by search engines (potentially an additional crawl per reply-able comment!) in exchange for enabling an awkwardly executed, likely underused, and non-essential feature for non-JS users.

I'd like to consider making comment reply JS-only.

Attachments (3)

22889.patch (4.8 KB) - added by joostdevalk 9 years ago.
Patch v1
22889.2.patch (5.8 KB) - added by markjaquith 9 years ago.
22889.3.patch (749 bytes) - added by joostdevalk 9 years ago.
Patch to add nofollow to ?replytocom links

Download all attachments as: .zip

Change History (40)

#2 @mdgl
12 years ago

Agreed that the current replytocom mechanism is really ugly and causes numerous difficulties, but it would be disappointing to lose the ability to handle threaded comments completely unless JS is enabled.

As far as I can see the underlying issue is how to get the variable comment_parent set for wp-comments-post.php when you submit a comment. It might be worth thinking through some other ways in which the user could get this variable set appropriately and without JS.

This and many other tickets are indicating that our comments system is probably long overdue for a revamp. I would suggest a phased approach over perhaps two releases: first clean up the underlying mechanisms and APIs; second create a better front-end with POST through the page template, overlayed with some nice AJAX support for those using JS.

#3 @banago
11 years ago

[...] for enabling an awkwardly executed, likely underused, and non-essential feature for non-JS users

Can't agree more. Using the HTML5 "data" attribute to have something like this, would work I think:

<a href="#" data=reply="{123}" title="Reply to this comment">Reply</a>

Or even move away from the anchor tag altogether.

Last edited 11 years ago by banago (previous) (diff)

#4 @joostdevalk
9 years ago

Can I bump this one? Would love to get this fixed. We default to disabling these non-JS links in our SEO plugins and never get complaints about it.

#5 @helen
9 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

I can move this into Future Release, as it's too late for 4.3. Mostly this needs a patch, I don't think there's any real opposition.

@joostdevalk
9 years ago

Patch v1

#6 @joostdevalk
9 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed
  • Owner set to joostdevalk
  • Status changed from new to assigned

#7 @markjaquith
9 years ago

I think we should keep the no-robots code. At least for a few releases. Those URLs might be floating around somewhere.

add_action( 'wp_head', 'wp_no_robots' );

#8 follow-up: @joostdevalk
9 years ago

In theory, the noindex shouldn't be needed because there's a canonical on the page that points to the right URL, especially as there are no internal links to those URLs anymore.

#9 in reply to: ↑ 8 @markjaquith
9 years ago

Replying to joostdevalk:

In theory, the noindex shouldn't be needed because there's a canonical on the page that points to the right URL, especially as there are no internal links to those URLs anymore.

Ah, the canonical point is a good one. That should handle it. I retract my suggestion.

#10 @joostdevalk
9 years ago

As this has a patch and is fairly straightforward, I'd love to milestone it to 4.3 :) @sam, @obenland?

#11 @markjaquith
9 years ago

Warning: Missing argument 3 for comment_form_title(), called in /Users/mark/Sites/wp.dev.git/src/wp-includes/comment-template.php on line 2286 and defined in /Users/mark/Sites/wp.dev.git/src/wp-includes/comment-template.php on line 1652

and

Notice: comment_form_title was called with an argument that is <strong>deprecated</strong> since version 4.3 with no alternative available. in /Users/mark/Sites/wp.dev.git/src/wp-includes/functions.php on line 3571

Also one of the changed PHPDoc lines removed an ending period.

@markjaquith
9 years ago

#12 @markjaquith
9 years ago

There. That should do it.

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


9 years ago

#14 @markjaquith
9 years ago

  • Milestone changed from Future Release to 4.3
  • Owner changed from joostdevalk to markjaquith
  • Status changed from assigned to accepted

#15 @markjaquith
9 years ago

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

In 33038:

Say goodbye to ?replytocom=123 links and their URL pollution.

  • Comment reply links continue to use JS as before.
  • ?replytocom=123 links are deprecated.

props joostdevalk
fixes #22889

#16 @markjaquith
9 years ago

In 33039:

Restore $noreplytext default value in comment_form_title()

props ocean90
see #22889

#17 @peterwilsoncc
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I’m reopening this as I have significant concerns about the implications around progressive enhancement/usability. I missed the slack conversation due to time-zones.

My concerns are:

  • the comment-reply JS is not enqueued by core when it’s needed, any themes that are not enqueuing the script properly will be incompatible with 4.3. This commit breaks backward compatibility
  • core functionality of any sites should be available without JavaScript. By switching to a JavaScript-only model, replies are removed from core-functionality with relatively little discussion
  • as I mention in #31590, the JavaScript for comment-replies can fail on large threads or slow connections for quick clicks on a reply link
  • in ideal network conditions this will have little effect. Users on slower networks are more likely to be affected than those in ideal conditions
  • the stated concern (crawling by search engines) is a concern for large sites with lots of comments. Such sites should be resourced.

I think removing progressive enhancement from the front end should be plugin territory. There is a filter in place to allow website owners to do this if they choose.

Removing progressive enhancement from 24% of the internet needs more discussion than a ticket milestoned an hour before the commit allows.

#18 @markjaquith
9 years ago

  • Milestone changed from 4.3 to Future Release

I don't agree with all these objections (or rather, don't care about all of them), but #31590 is a definite blocker.

#19 @markjaquith
9 years ago

Reverted for now: [33042]

#20 @joostdevalk
9 years ago

Ok if we're going to revert, I'm going to propose that we add a nofollow to these replytocom links. Should make Google's crawling slightly more sane.

@joostdevalk
9 years ago

Patch to add nofollow to ?replytocom links

#21 @SergeyBiryukov
9 years ago

In 33047:

Restore rel='nofollow' for comment reply links to reduce extra crawling by search engines.

This reverts [16230], which didn't have enough review at the time of commit.

props joostdevalk.
see #22889, #18547, #16881.

#22 @peterwilsoncc
9 years ago

Thanks, [33047] is a great pragmatic mid-point. I'd like to see this closed as fixed with this patch.

It solves the stated problem of excessive crawling by bots without penalising the JavaScript impaired. Google's SEO advice is to make sites for humans before bots, we've done that.

#23 @dshanske
8 years ago

Can't we resolve this with #34106 ?

#24 @webliberty
7 years ago

Is it not worth returning to this question? Google does not always follow the rules of robots.txt and rel='nofollow', considering this a recommendation, rather than a strict requirement.

In the plugin Yoast SEO has a fix, maybe it should be replaced as well ? on #.

#25 @peterwilsoncc
7 years ago

  • Keywords close added

FWIW, the bug that led to reverting above has being fixed in #31590 [42360] .

This has been sitting dormant for three years, let's close as fixed with [33047].

#26 @peterwilsoncc
7 years ago

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

#27 @peterwilsoncc
7 years ago

  • Milestone Future Release deleted

#28 @joostdevalk
7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I want to really fix this. The nofollow is just a stop-gap and I'm seeing far too much of these URLs in Google's index. @markjaquith @SergeyBiryukov could you help get this fixed properly?

#29 @SergeyBiryukov
7 years ago

  • Milestone set to 5.0
  • Owner changed from markjaquith to SergeyBiryukov
  • Status changed from reopened to accepted

#30 @peterwilsoncc
7 years ago

@joostdevalk

Are you able to provide the search link for URL fragments? My google skills are not as good as yours.

I'd like to see if big search ignores the noindex, follow tag & canonical reference core adds to the replytocom HTML header or if it's the result of a bug in a theme or plugin.

My arguments agains this change are largely unchanged from comment 17 above. Removing progressive enhancement places developer convenience above the users is counter best-practice, and in this case a very first world centric change.

#31 @juslintek
6 years ago

After removing it commentator specified 'Reply to %s' message doesn't work anymore:
site/web/wp/wp-includes/comment-template.php:1850

<?php
/**
 * Display text based on comment reply status.
 *
 * Only affects users with JavaScript disabled.
 *
 * @internal The $comment global must be present to allow template tags access to the current
 *           comment. See https://core.trac.wordpress.org/changeset/36512.
 *
 * @since 2.7.0
 *
 * @global WP_Comment $comment Current comment.
 *
 * @param string $noreplytext  Optional. Text to display when not replying to a comment.
 *                             Default false.
 * @param string $replytext    Optional. Text to display when replying to a comment.
 *                             Default false. Accepts "%s" for the author of the comment
 *                             being replied to.
 * @param string $linktoparent Optional. Boolean to control making the author's name a link
 *                             to their comment. Default true.
 */
function comment_form_title( $noreplytext = false, $replytext = false, $linktoparent = true ) {
        global $comment;

        if ( false === $noreplytext ) $noreplytext = __( 'Leave a Reply' );
        if ( false === $replytext ) $replytext = __( 'Leave a Reply to %s' );

        $replytoid = isset($_GET['replytocom']) ? (int) $_GET['replytocom'] : 0;

        if ( 0 == $replytoid )
                echo $noreplytext;
        else {
                // Sets the global so that template tags can be used in the comment form.
                $comment = get_comment($replytoid);
                $author = ( $linktoparent ) ? '<a href="#comment-' . get_comment_ID() . '">' . get_comment_author( $comment ) . '</a>' : get_comment_author( $comment );
                printf( $replytext, $author );
        }
}

Haven't figure it out yet, how to pass this variable via ajax. Any non-hardcore hacks anyone come up regarding this?

#32 @pento
6 years ago

  • Milestone changed from 5.0 to 5.1

#33 follow-up: @pento
6 years ago

  • Milestone changed from 5.1 to Future Release

This ticket needs further investigation on the current impact, and if search engines aren't obeying the canonical header, why not?

#34 in reply to: ↑ 33 @jonoaldersonwp
5 years ago

Replying to pento:

This ticket needs further investigation on the current impact, and if search engines aren't obeying the canonical header, why not?

Because canonical is a hint, not a directive. If there are conflicting signals, they'll ignore it. The only way to fix this is to actually fix it.

#35 @hellofromTonya
21 months ago

Hello all,

Checking in as this ticket's progress stalled 5 years ago.

What's the status of "needs further investigation on the current impact"?

Fast forward from when this ticket was opened: Is there still a need today?

#36 @johnbillion
21 months ago

  • Keywords close removed

#37 @peterwilsoncc
21 months ago

I think this is what is needed to proceed:

  • Have WP enqueue the script if needed:
    <?php
    if ( is_singular() && comments_open() && get_option('thread_comments') )
      // Maybe check the count of comments too, can't reply if there are none.
      wp_enqueue_script( 'comment-reply' );
    ?>
    
  • a11y review to ensure the comment reply script is fully accessible (I don't think it makes any announcements as to the changed content, should it?)

A quick look at google for inurl:?replytocom= wasn't showing a great deal of results, several of those I did see were including the parameter in the canonical meta tag, so it's possible the few results are from server misconfiguration.

If the JS is accessible, I'm less not-keen on removing the progressive enhancement now than I was six years ago. I'd still prefer WP not to require sites enforce the use JavaScript on the front end though, as we can't be sure that's suitable in each case.

Note: See TracTickets for help on using tickets.