snarfed / granary

💬 The social web translator
https://granary.io
Creative Commons Zero v1.0 Universal
450 stars 57 forks source link

Duplicate in-reply-to links on Tweets #46

Closed kylewm closed 9 years ago

kylewm commented 9 years ago

In the last week or so, I've noticed Twitter posts have started showing up with duplicated in-reply-to links:

<article class="h-entry h-as-note">
  <span class="u-uid">tag:twitter.com:653670712104738816</span>
  <time class="dt-published" datetime="2015-10-12T20:36:57+00:00">2015-10-12T20:36:57+00:00</time>
  <div class="h-card p-author">
    <div class="p-name"><a class="u-url" href="https://kylewm.com">Kyle Mahan</a></div>
    <img class="u-photo" src="https://twitter.com/kylewmahan/profile_image?size=original" alt="">
  </div>
  <a class="u-url" href="https://twitter.com/kylewmahan/status/653670712104738816"></a>
  <div class="e-content p-name">
  <a href="https://twitter.com/WeWantPlates">@WeWantPlates</a> <a href="https://twitter.com/BethFad91">@BethFad91</a> oh good, it looks like most of the paint has already been scraped off by other people's utensils
  </div>
  <a class="u-in-reply-to" href="https://twitter.com/WeWantPlates/status/653649456454365184"></a>
  <a class="u-in-reply-to" href="https://twitter.com/WeWantPlates/status/653649456454365184"></a>
</article>

I haven't looked into what's causing this yet

kylewm commented 9 years ago

This change started including inReplyTo in both the object and context: https://github.com/snarfed/granary/commit/b0ad8c37128c815936a7193ddc2959c0cbcd72f5

microformats2 conversion collects inReplyTo from both, so we predictably get doubles. The microformats conversion should probably pull from object.get('inReplyTo', context.get('inReplyTo', [])) instead of adding them together

snarfed commented 9 years ago

ooh good catch, thanks! sgtm.

snarfed commented 9 years ago

thinking about this more, another idea would be to merge and de-dupe both. they're probably only different in practice very rarely, though, and i expect never when we generated the AS ourselves - the common case - so probably not worth it.

kylewm commented 9 years ago

I don't think context and inReplyTo are even defined in AS1, and agree it's probably not worth going out of our way to support arbitrary input on those.