Reviewing how the twitter module use the oauth module I have seen that the only use that it mades from oauth is the http://code.google.com/p/oauth/ library. (twitter.lib.php class TwitterOauth that uses OAuthSignatureMethod_HMAC_SHA1, OAuthConsumer classes from OAuth.php library).

There is no use of any oauth module API or oauth function or oauth module feature.
Indeed if we included this library in the twitter module itself we should avoid installing unnecesary modules but as it could drive to conflicts with other modules doing the same I agree that it sound better to use the existing oauth_common module that centralizes this library.

This patch only change the loading library from the oauth 2.x to the new oauth_common 3.x version.

I couldn't see any incompatibility from using the 3.x or 2.x because there is no any use from the modules code. Common_oauth use a oauth project library from r1231 instead than r666 from 2.x, http://code.google.com/p/oauth/source/list?path=/code/php/OAuth.php&star...
so it fixes some bugs.

CommentFileSizeAuthor
#12 twitter_3.x_oauth_3.x-125510-1.patch4.79 KBsteinmb
#3 twitter_3.x_oauth_3.x-125510.patch3.88 KBsplash112
twitter_3.x_oauth_common.patch3.82 KBjcmarco

Comments

michaek’s picture

michaek commented

This looks totally awesome. I'll apply, test, and push as soon as I get a chance!

I honestly hadn't looked into this issue at all, because I'd assumed if the previous maintainers hadn't done it, it must be a thorny issue, and I should wait until I had some time to spend on it. Thanks for taking this on!

splash112’s picture

splash112 commented

Testing!
Btw, change the dependency to oauth 3.x seems to make sense, as it is now the recommended version.

splash112’s picture

splash112 commented
StatusFileSize
new twitter_3.x_oauth_3.x-125510.patch3.88 KB

Seems works fine here. Had to patch by hand because jcmarco's test didn't apply.
So attached new patch against latest 3.x dev.

michaek’s picture

michaek commented

Thanks, @splash112. I'll test it and get it rolled into a release. This issue is a big deal!

splash112’s picture

splash112 commented

Welcome, keep up the goods work with this very nice module!

splash112’s picture

splash112 commented

Btw, for oauth you will need to completely delete the old module before uploading the 3.x module.

michaek’s picture

michaek commented

Thanks, I'd do that anyway, as it's standard for upgrading modules. (Plus, it's what `drush dl` does. :) )

michaek’s picture

michaek commented
Status: Needs review » Closed (fixed)

Tested. This will shortly be released in the dev version. I would love for a few people to test it in the meantime, but I'll probably trigger another beta release in a few days! Thanks to both of you!

socialnicheguru’s picture

socialnicheguru commented

I can confirm that this works with Oauth 3

socialnicheguru’s picture

socialnicheguru commented

has this been committed

steinmb’s picture

steinmb commented
Status: Closed (fixed) » Needs review

Not able to find any commits, reopening.

steinmb’s picture

steinmb commented
StatusFileSize
new twitter_3.x_oauth_3.x-125510-1.patch4.79 KB

#3 does not apply cleanly anymore, re-rolling, though patch untested.

killua99’s picture

killua99 commented
Status: Needs review » Reviewed & tested by the community

tested, works correctly.

steinmb’s picture

steinmb commented
Status: Reviewed & tested by the community » Needs work

Found #935196: Module should raise notices when it is not configured (preventing Twitter authentication failed message) in the issue q. We prob. should throw some warning that wrong version of OAuth is installed?

killua99’s picture

killua99 commented

Yes but that is another issue, The first point of this issue is to make a patch to use the Oauth v-3.x and that task is done. Now we need to handle the protocol version, and another things more.

But for me this issue are "reviewed & tested by the community"

steinmb’s picture

steinmb commented
Status: Needs work » Reviewed & tested by the community

Just checking :) Did also a quick test and think this is RTBC, yay. Oh happy days when both branches are using the same OAuth version! Let's get this commited!

juampynr’s picture

juampynr commented
Status: Reviewed & tested by the community » Fixed

Tests performed against patch #12:

  • Add a Twitter account without OAuth: OK.
  • Add a Twitter account with OAuth: OK.
  • Sign in with Twitter with an already added Twitter account: OK.
  • Sign in with Twitter and then relate the Twitter account with a user: OK.
  • Post to Twitter when creating a page: OK.
  • Post to Twitter through an action: OK.
  • Post to Twitter through a rule: KO. However this occurs without the patch, so it is a different error and therefore I will search for it in the issue queue or create an issue.

I have committed a slightly modified version of the last patch given at comment #12.
http://drupalcode.org/project/twitter.git/commitdiff/1fb8848a68a5c1cc050....
I have also updated OAuth requirements at README.txt at http://drupalcode.org/project/twitter.git/commitdiff/a9a81e19afe146a0c7f....

By the end of the week I will release a new version.

Many thanks to everyone for the hard work.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.