Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

serve facebookexternalhit user agent with spiderable #411

Closed
wants to merge 2 commits into from

Conversation

ayal
Copy link
Contributor

@ayal ayal commented Oct 20, 2012

This change will make Spiderable serve facebookexternalhit user agent.

Using Spiderable after this change would enable one to manipulate the head tag in "client code", which would then actually be served to the Facebook bot when it comes looking for "open graph meta tags" (i.e https://developers.facebook.com/docs/opengraphprotocol/)

The main code-change is adding "facebookexternalhit" to the condition under-which the phantomjs process is initiated. Other than that there are two other complementing code-changes:

  1. A rewrite of the code that re-assembles the request url without the "escaped_fragment" query param. This is needed because in the current version of Spiderable the code assumes there is always a query string (which might not be the case in the proposed version)
  2. A small bug-fix to the regex test for html validity of the phantomjs response page (html openning tag might contain attributes, especially after client manipulation, which is the case here)

To test this:

  1. Create a new meteor app
  2. On client startup, add a meta tag to head. Something like:
$("head").append('<meta property="og:title" content="spidy" />');
  1. Optionally try having the content arrtibute be something you fetch from the DB in order to simulate how a user might use this. Preferably don't use autopublish, and put the code in the callback of the "subscribe" call to the data you're using. This is to make sure "_allSubscriptionsReady() === false" until you've done your manipulation.
  2. Test this with Facebook Debugger: https://developers.facebook.com/tools/debug (notice there's an option at the bottom to actually see what your server served Facebook)

@75lb
Copy link
Contributor

75lb commented Oct 25, 2012

something like this will come in useful for me at some point.. for the time being i'm serving Open Graph objects from the public folder.. Facebook gets the metadata it needs, a user visit gets redirected into the main app..

@ayal
Copy link
Contributor Author

ayal commented Oct 25, 2012

@75lb, while this is being reviewed - you may be interested in headly:
https://github.com/ayal/headly

@n1mmy
Copy link
Contributor

n1mmy commented Oct 26, 2012

Thanks for the patch, @ayal! This looks good.

One change I'd request, though: can you pull the 'facebookexternalhit' constant out into a list of user-agents to check for. So the test is something like "is the user-agent in this list" not "is the user-agent facebook". Then it's easier to add more later, and more clear what is going on.

Also, you'll need to sign the Meteor contributor agreement before we can merge your patch. http://contribute.meteor.com/. Thanks!

@ayal
Copy link
Contributor Author

ayal commented Oct 28, 2012

@n1mmy, see my latest commit with the "agents" list.
Also, I was unable to submit the contribute form. I signed with GitHub successfully, but filling the form always just results in: "You must sign in with GitHub, all fields are required and you must type "I AGREE" in the last box." (which I did)

@ayal
Copy link
Contributor Author

ayal commented Oct 29, 2012

ok @n1mmy, I managed to sign the contributor agreement.

@n1mmy
Copy link
Contributor

n1mmy commented Oct 29, 2012

Great, thanks @ayal. Will test and merge.

@n1mmy n1mmy closed this in dcdc8c0 Oct 30, 2012
@n1mmy
Copy link
Contributor

n1mmy commented Oct 30, 2012

Merged. Thanks!

StorytellerCZ pushed a commit that referenced this pull request Sep 18, 2021
Update babel-runtime to version 6.20.0 🚀
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants