wordpress-webmention icon indicating copy to clipboard operation
wordpress-webmention copied to clipboard

Add Comment Permalink Support

Open dshanske opened this issue 7 years ago • 22 comments

@pfefferle You were working on this in Webmentions for Comments. Since Webmention now includes several templates, I want to see about merging in this code, but using the query variable comment over replytocom.

Other supporting information: https://core.trac.wordpress.org/ticket/34106

dshanske avatar Mar 12 '17 16:03 dshanske

yes please.

miklb avatar Mar 14 '17 19:03 miklb

Working on this in a branch... https://github.com/dshanske/wordpress-webmention/tree/comments

So far, there are comment templates for single and archive that need to be designed, but the functionality works. After the permalink support is better developed, will trigger webmentions based on it. Any feedback appreciated.

@pfefferle Could you note this as assigned to me?

dshanske avatar Apr 16 '17 18:04 dshanske

I am not sure if it is a good idea to implement a permalink structure for comments. If it will be implemented into core, we should wait until the feature is implemented and should use the reply link in the meantime. Otherwise we have to keep a lot of code to be backwards compatible if the final implementation differs from the Webmention implementation.

I also don't like the Webmention comment templates, because they will look ugly in most themes. Thats why I didn't implement the threaded comments Webmention code into the core plugin.

pfefferle avatar Apr 16 '17 19:04 pfefferle

With the fragment changes made, I can hook onto those now, where previously that wasn't possible and see if that works to address the threading issue.

dshanske avatar Apr 16 '17 19:04 dshanske

I know you were using ?replytocom, but would #comment-123 work?

dshanske avatar Apr 16 '17 19:04 dshanske

OK, that is a valid point, but then I would recommend to start implementing the hash support into the threaded comments plugin. If that works, we should clean up the code and merge it into the core plugin. And if all that works fine, we can think about the permalink structure.

pfefferle avatar Apr 16 '17 19:04 pfefferle

Sold. Will fork and enhance.

dshanske avatar Apr 16 '17 20:04 dshanske

cool thanks!

pfefferle avatar Apr 16 '17 20:04 pfefferle

The goal was to try to bring things together as much as possible, so if working on it separately before merge is the best way to accomplish that, we are moving toward unification.

dshanske avatar Apr 16 '17 20:04 dshanske

I have been brainstorming about this for a few days. If we tune the receiver in the main plugin to look for comment fragment IDs and verify them against the comment itself, that is harder to do in the split plugin without replacing the verification functionality a bit.

On the sender side, this is much easier. Just add the Fragment.

But if above was implemented, that would work without a definitive comment permalink if the comment was marked up properly on the page. It would create a problem working with pagination though. Since many themes paginate Comments, still stuck on that part of the problem.

Short of taking over the comment template and possibly JS, not sure best method

dshanske avatar Apr 20 '17 16:04 dshanske

...and it could cause problem with other sides that do not support fragments. fragments are not part of the main spec so we should only use it as an extension/fallback.

pfefferle avatar Apr 20 '17 18:04 pfefferle

Not 100% sure how many endpoints support query strings. I should poll around. But this requires more thought.

dshanske avatar Apr 21 '17 05:04 dshanske

As the threaded comment system was built into the plugin, @pfefferle we need to revisit this I think

dshanske avatar Aug 04 '19 17:08 dshanske

Why? Any issues?

pfefferle avatar Aug 04 '19 17:08 pfefferle

Revisit the question of comments having pernalinks as the reason was listed as due problems with a feature now merged.

dshanske avatar Aug 04 '19 17:08 dshanske

I'm reading through issues to figure out how to close them

dshanske avatar Aug 04 '19 17:08 dshanske

In the best case you do not have to know about the permalink

pfefferle avatar Aug 04 '19 18:08 pfefferle

I know you were using ?replytocom, but would #comment-123 work?

I recently added #comment-123 support to my webmention implementation, but I don't think (?) the Webmention plugin currently supports it (on the receiving end).

What I myself did, is whenever I receive a mention with a source of, e.g., https://example.org/a/post#comment-123, is fetch that page, and then parse only the element with id=comment-123 when looking for microformats.

And when the target is, e.g., https://example.net/another/post#comment-245, well, I simply assume they're replying to the comment with ID 245, even without a ?replytocom query argument.

Anyway, what I think happens when I send a mention with such a source to a site that has Webmention enabled, is it may discard the fragment part of the URL and thus picks up the wrong microformats (the main post rather than the comment).

But I'm not sure, hence this message ;-)

So, I actually have two questions:

  1. Should I add ?replytocom instead, when sending out webmentions, just to be sure? Is there a sort of agreed way to do this?
  2. Will Webmention recognize "remote fragments," i.e., parse "only" the "correct" part of the page? I thought maybe php-mf2 would already do this, but it looks like it doesn't, which is why, in my code, I do it explicitly (by feeding it only the "correct" element). E.g., looking at https://pin13.net/mf2/?url=https%3A%2F%2Fjan.boddez.net%2Fnotes%2F9a5f268a52%23comment-1841, I see that it parses the whole page and not just the comment with ID 1841.

janboddez avatar Jan 01 '24 13:01 janboddez

In my code, when sending, I basically just do (i.e., get_comment_link() adds the #comment-123 bit for me):

$response = wp_remote_post(
  esc_url_raw( $endpoint ),
  array(
    'body' => array(
      'source' => $obj instanceof \WP_Post ? get_permalink( $obj->ID ) : get_comment_link( $obj->comment_ID ),
      'target' => $url,
    ),
  )
);

When receiving a mention, I essentially do:

// Look for a target URL fragment (and possible parent comment).
$fragment = wp_parse_url( $target, PHP_URL_FRAGMENT );
if ( ! empty( $fragment ) && preg_match( '~^comment-\d+$~', $fragment ) ) {
  $parent = get_comment( str_replace( 'comment-', '', str_replace( 'comment-', '', $fragment ) ) );
}

And then if it finds a parent I set that as the new comment's parent (thinking of, I might have to first check if their comment_post_IDs match). I believe this plugin does the same, but with the replytocom parameter. I was wondering if using a URL fragment (or both, for backward compatibility) doesn't make "more sense"?

janboddez avatar Jan 01 '24 13:01 janboddez

And before I actually look for microformats, I do something like this (in reality, there's probably all sorts of ways this can go wrong, but it's something):

$fragment = wp_parse_url( $url, PHP_URL_FRAGMENT );
if ( ! empty( $fragment ) ) {
  // If the URL contains a fragment, parse only the corresponding
  // page section.
  $xpath = new \DOMXPath( $dom );

  foreach ( $xpath->query( "//*[@id='$fragment']" ) as $el ) {
    $content = $dom->saveHTML( $el );
    break;
  }
}

$mf2 = Mf2\parse( $content, $url );

I know I should probably do a PR or something instead rather than dump code snippets here in the comments, but I'm not sure if you'd be interested. Plus, I'm not super familiar with the codebase; thought I'd just ask/drop this here first.

janboddez avatar Jan 01 '24 14:01 janboddez

Just noticed :-) this plugin does save a webmention_target_fragment if there is one, although I don't think it really uses it when parsing source pages?

Another thing I was wondering about: if one WordPress site sends out a webmention for a comment (as a reply to another comment that originated as a mention from another WP install) and it adds the replytocom parameter, isn't there a risk that it will map to an existing comment on that other site? That the other site, when it receives this reply, will set a completely wrong comment_parent? Couldn't that potentially mess up comment threading? (Unless I'm reading the code wrong, which is ... very much possible.) :-D

janboddez avatar Jan 09 '24 17:01 janboddez

@janboddez it only checks the target for the replytocom and we check if the target has the same host as the blog, so this should not be a problem.

pfefferle avatar Jan 10 '24 09:01 pfefferle