Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#22422 closed defect (bug) (fixed)

Fix the Tumblr Importer

Reported by: westi's profile westi Owned by: otto42's profile Otto42
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.4.2
Component: Import Keywords: audit needs-testing
Focuses: Cc:

Description

We don't currently have a working Tumblr Importer and so we should remove it from Tools > Import

Ideally we should drive the list from an API on WordPress.org but we have more important things to focus on for 3.5

Attachments (5)

no-tumblr-for-now.diff (914 bytes) - added by westi 11 years ago.
Remove the importer from the list of popular importers
import.php (6.0 KB) - added by ryansatterfield 11 years ago.
removed line 41 that supported tumblr imports
import.2.php (6.0 KB) - added by ryansatterfield 11 years ago.
removed tumblr from line 41 using the latest nightly update. I don't know if there were any changes to this file, so I thought it was safer to reupload it.
tumblr.patch (1.3 KB) - added by dllh 11 years ago.
Minor diffs failing to make drafts work
keys-from-api.patch (6.2 KB) - added by dllh 11 years ago.
Fetches consumer/secret keys from an api

Download all attachments as: .zip

Change History (47)

#1 @westi
11 years ago

  • Owner set to westi
  • Status changed from new to accepted

@westi
11 years ago

Remove the importer from the list of popular importers

#2 @dllh
11 years ago

  • Cc daryl@… added

#3 @SergeyBiryukov
11 years ago

Ideally we should drive the list from an API on WordPress.org

Related: #18977

#4 @nacin
11 years ago

  • Priority changed from normal to high

We should really just aim to fix this importer before 3.5 is released. I know it's a pain because of their API changes, but it is rather embarrassing to not have one.

Happy to leave this open as a tracking ticket as we close in on 3.5's release date, as well as fix up and add #18977 to core now.

#5 follow-up: @JustinSainton
11 years ago

Happy to volunteer the time to fix the importer if no one on the core team has time.

#6 in reply to: ↑ 5 @westi
11 years ago

Replying to nacin:

We should really just aim to fix this importer before 3.5 is released. I know it's a pain because of their API changes, but it is rather embarrassing to not have one.

Happy to leave this open as a tracking ticket as we close in on 3.5's release date, as well as fix up and add #18977 to core now.

I would love for this to be fixed I just didn't want us to release with the current experience because it is pretty bad.

I know both Otto and Daryl (dllh) have been working on this but my understanding is that the .org Tumblr importer still has some road blocks that are blocking it's re-release.

Replying to JustinSainton:

Happy to volunteer the time to fix the importer if no one on the core team has time.

Thanks, best thing to do is to talk with Otto/Daryl and see how you can help them.

#7 @nacin
11 years ago

  • Owner changed from westi to nacin

#8 @dllh
11 years ago

As I understand it, one hurdle is that users will have to create their own Tumblr app to get a token in order to connect, which is a pretty high barrier for lots of users. I think the other hurdle (from chatting briefly with Otto) is that the code's just not quite working yet. We've got it working pretty well on wordpress.com and I'm happy to try out Otto's code and see if I can spot any issues; it's just a matter of finding the time.

@ryansatterfield
11 years ago

removed line 41 that supported tumblr imports

#9 @ryansatterfield
11 years ago

  • Cc r@… added
  • Keywords has-patch 2nd-opinion added

I fixed it. I know I am not supposed to post the entire file, but it's late and I forget the proper etiquette for trac and how I am supposed to submit the changes. I uploaded the fix. It works fine on my install.

@ryansatterfield
11 years ago

removed tumblr from line 41 using the latest nightly update. I don't know if there were any changes to this file, so I thought it was safer to reupload it.

#10 @SergeyBiryukov
11 years ago

  • Keywords needs-patch added; has-patch removed

We should really just aim to fix this importer before 3.5 is released.

#11 @nacin
11 years ago

  • Keywords 2nd-opinion needs-patch removed
  • Priority changed from high to highest omg bbq
  • Summary changed from Remove the Tumblr Importer from Tools > Import to Fix the Tumblr Importer

Pivoting. This should be high priority. Let's get this thing shipped! Otto has committed what he has: http://plugins.svn.wordpress.org/tumblr-importer/branches/newapi/. He has requested additional eyeballs to make it all work.

westi and I were talking about just shipping with our oAuth keys in the code, as they're not really "secret", and not requiring the user to create an application. The next-best alternative is to actually proxy this stuff through api.wordpress.org.

Asking users to create their own application is not a very good experience. To psychoanalyze potential users: going from Tumblr to WordPress, users probably want more control over their content and more power over their site, but may not be sold on the transition and may want to play around with WordPress. Just because they want more power, doesn't mean they're savvy enough to not drop off during the complexities of an oAuth handshake. Forcing them to create an application is probably going to create too high of a barrier and reduce the number of successful imports.

#12 follow-up: @nacin
11 years ago

Note that the newapi branch handles app creation, the oAuth handshake, and lists the users blogs for them. The current blocker is fixing the import process, which is broken. dllh, maybe you could lend a hand on this in particular?

After it "works", we should figure out the course of action for the application and handshake.

#13 in reply to: ↑ 12 @dllh
11 years ago

Yep, glad to take a look. Have been meaning to nag Otto for his code, but if it's committed in a branch, I can peek sometime soonish.

#14 @Otto42
11 years ago

  • Cc Otto42 added; otto42 removed

#15 follow-up: @dllh
11 years ago

Well, so far I can't get the connection with Tumblr to work. After clearing account info from options, etc., I consistently get a "Not Found" error from Tumblr's authorize url. So that's fun.

#16 in reply to: ↑ 15 @dllh
11 years ago

Replying to dllh:

Well, so far I can't get the connection with Tumblr to work. After clearing account info from options, etc., I consistently get a "Not Found" error from Tumblr's authorize url. So that's fun.

urlencode() FTW.

#17 @dllh
11 years ago

In 626309 I've gotten us over the main hurdle. We can do an import now. I haven't tested much at all yet, and I need to do a general audit of the code to make sure things that work at present on wpcom have been ported over into this version too.

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

@dllh
11 years ago

Minor diffs failing to make drafts work

#18 @dllh
11 years ago

I made several more improvements tonight including some fixes to importing audio and drafts. There's still some cleanup to do along with the whole issue of how to handle the auth (I'm a fan of not making people create apps), but it'd be great if somebody other than me coud test with a real tumblr that'll test more cases than my dummy blog.

#19 @dllh
11 years ago

  • Keywords needs-testing added

#20 @nacin
11 years ago

  • Owner changed from nacin to Otto42
  • Status changed from accepted to assigned

@dllh
11 years ago

Fetches consumer/secret keys from an api

#21 @dllh
11 years ago

As a little proof of concept, I've added keys-from-api.patch, which replaces the "go create your own app" rigamarole with bits that fetch consumer and secret keys from the api if not stored in a transient.

From a user perspective, this is a much improved experience, though it's maybe not 100% desirable to have the keys world readable. Still, it may be a better alternative to shipping them in the plugin itself, and it'd allow the keys to be swapped out pretty quickly if needed.

It's sort of a compromise between putting the onus on the poor user and shipping keys in software, without going to the (potentially painful) extreme of making an actual proxy for the auth.

#22 @Otto42
11 years ago

Tumblr doesn't have a read only permissions model. Exposing the keys will give anybody full access to the tumblr account of anyone who uses the importer.

#23 @georgestephanis
11 years ago

As per Tumblr's License Agreement [ http://www.tumblr.com/policy/en/api ] section 3b, distributing the API key does violate their terms of service, and they may revoke access for that reason specifically.

Have we tried to reach out to them? Get special permission for this?

Worst case scenario, the API keys could be kept private, and the Tumblr importer could be incorporated into Jetpack, which reads to me as more in keeping with the spirit of their Licensing Agreement?

#24 @nacin
11 years ago

I think we should proxy things through WordPress.org.

#25 @MZAWeb
11 years ago

I just imported 550 posts from two different tumblrs without a hitch, and it was quite fast.

One comment, though. The "We have saved some information about your Tumblr account in your WordPress database...." message is confusing on first load, before I actually inserted any information about my Tumblr. I'd say we should hide that if there's no stored info.

#26 follow-up: @MZAWeb
11 years ago

I'm afraid I spoke too soon. Post of type Videos are missing the actual video. Post in WP is created with the text and meta, but the video itself is missing.

#27 @MZAWeb
11 years ago

  • Cc wordpress@… added

#28 in reply to: ↑ 26 ; follow-up: @dllh
11 years ago

Replying to MZAWeb:

I'm afraid I spoke too soon. Post of type Videos are missing the actual video. Post in WP is created with the text and meta, but the video itself is missing.

What kind of videos? I tested this with a .mp4 movie that came in without issue. Is the video missing from the media library or just not inserted into the post?

#29 in reply to: ↑ 28 @MZAWeb
11 years ago

Replying to dllh:

What kind of videos? I tested this with a .mp4 movie that came in without issue. Is the video missing from the media library or just not inserted into the post?

Embeds.
Tumblr side: http://screenshots.mzaweb.com/hIoI WP side: http://screenshots.mzaweb.com/hIoM

#30 @johnbillion
11 years ago

I just ran an import from my Tumblr blog. A few issues:

  1. Video posts which use videos from YouTube/Vimeo or which contain a raw embed code are missing the actual video. I suspect this is what MZAWeb also reported.
  2. Chat posts do not get the 'chat' post format applied.
  3. Image posts are missing any text that accompanies the image.
  4. Quote posts aren't formatted very well. The quote and the source are simply inserted inline. Might be better to wrap the quote in a <blockquote>?

#31 follow-up: @dllh
11 years ago

I've fixed the chat post format issue in 628490, the quote formatting issue in 628492, and the image caption issue in 628493.

I've so far been unable to reproduce the video embed issue.

#32 in reply to: ↑ 31 @MZAWeb
11 years ago

Replying to dllh:

I've so far been unable to reproduce the video embed issue.

I can create an oauth access for you to my blog, if you think that may help you debug. If so, drop me an email to daniel @ my username

.com

#33 @dllh
11 years ago

I can now repro video issues with Jetpack disabled thanks to some live debugging with @MZAWeb.

#34 follow-up: @dllh
11 years ago

Adding iframe to the whitelisted tags for kses gets around the video issue, but I'm not sure what the appropriate way to do this is or whether it's advisable at all. I guess I could check for (youtube|vimeo) and add to the whitelist per post, then remove from kses after each post, but I can't help thinking there's a better way. Feedback welcome.

#35 follow-up: @johnbillion
11 years ago

For sites that are in the list of supported oEmbed providers we should just insert the video URL and let oEmbed embed do its thing.

#36 in reply to: ↑ 35 @DrewAPicture
11 years ago

Replying to johnbillion:

For sites that are in the list of supported oEmbed providers we should just insert the video URL and let oEmbed embed do its thing.

+1. Work smarter not harder :)

#37 @ryansatterfield
11 years ago

Kinda odd idea, but why doesn't WordPress just have someone enter the URL of the information they want imported, have a spider crawl that URL and import the cache. I know some sites have a no spider, no scraping rule, but some don't. Just food for thought for future import tickets.

#38 @nacin
11 years ago

  • Keywords audit added
  • Priority changed from highest omg bbq to normal

#39 in reply to: ↑ 34 ; follow-up: @Otto42
11 years ago

Replying to dllh:

Adding iframe to the whitelisted tags for kses gets around the video issue, but I'm not sure what the appropriate way to do this is or whether it's advisable at all. I guess I could check for (youtube|vimeo) and add to the whitelist per post, then remove from kses after each post, but I can't help thinking there's a better way. Feedback welcome.

The original importer code I wrote handled this correctly for the specific case of YouTube, by turning it into an oEmbed. Looks like that code got lost somewhere along the way...

http://plugins.trac.wordpress.org/browser/tumblr-importer/trunk/tumblr-importer.php#L786

#40 in reply to: ↑ 39 @dllh
11 years ago

Replying to Otto42:

http://plugins.trac.wordpress.org/browser/tumblr-importer/trunk/tumblr-importer.php#L786

I'll see about resurrecting that over the weekend if nobody beats me to it.

#41 @dllh
11 years ago

Think I've got the YouTube and Vimeo things fixed. I've tagged a new beta as newapi-beta-2 and commented at http://make.wordpress.org/core/2012/11/21/if-you-have-a-tumblr-blog-can-you/ to let any latecomers know that there's a new zip to test.

#42 @nacin
11 years ago

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

Per IRC discussion with dllh and westi, Otto42 is in the process of releasing this new plugin. (If there are still issues, it is better than what we have.) Go team!

Note: See TracTickets for help on using tickets.