codebykat / wp-post-by-email

Post By Email plugin for WordPress
GNU General Public License v2.0
31 stars 11 forks source link

Support embedded images #5

Open codebykat opened 10 years ago

codebykat commented 10 years ago

Currently, images are only supported as attachments. Many email clients allow you to embed images within the body of the message; this functionality should be supported by the plugin.

http://wordpress.org/support/topic/images-not-loading-28 http://wordpress.org/support/topic/pictures-not-loading-1

wubbahed commented 10 years ago

I was having this same issue around embedded image support, but was able to fix it by upgrading to the latest version of Horde (2.24.2 at the time of this writing)

codebykat commented 10 years ago

Thanks @wubbahed! That's an option for some, but I believe the most recent version of Horde doesn't support PHP 5.2, so I can't just move to that. In the next version I'm thinking of switching to the built-in IMAP library for older versions of PHP.

nikolov-tmw commented 9 years ago

@wubbahed - how exactly did you upgrade Horde? I'm a bit lost at trying to do the same, since we'll be receiving images from multiple authors and we'd like to support both embedded and attached images(attachments work without a problem) to make it easier to post.

wubbahed commented 9 years ago

@nikolov-tmw - in this plugin, there's a folder called /include/Horde/ which contains six folders, each representing a Horde module. For each one of these, you can download the latest module here: http://www.horde.org/libraries

It's been a while since I did this so not sure if there's any additional tweaking that needs to be done, but if so, it should only involve tweaking file paths.

nikolov-tmw commented 9 years ago

@codebykat and @wubbahed - do you mind discussing here how to do the inlined image replacement(the pull request referenced above successfully pulls inlined images into the Media Library, but it doesn't replace them in the content).

What I did as a patch for right now was to store an array of cid's that got imported to the media library and then used the following regular expression to capture all img tags from the post content.

You can see an example here: https://gist.github.com/nikolov-tmw/3b1c068ada7010296652

What I'm not sure how to approach is whether to wrap the img tags in anchors that link to the full size of the image or not. And if we do, how to make it overridable via filters.

In my case, we needed the images to link to the full-size image and that's why I implemented it that way, but I'd be interested to hear your opinions as well.

Nikola

codebykat commented 9 years ago

OMGGGG @nikolov-tmw this is really cool! :open_mouth: :fireworks: :ok_woman:

Merged in the sideloading stuff just now. Love that you also set the featured image - that also fixes #13. :moneybag:

WP has options for image defaults: image_default_size and image_default_link_type. We could use those and have folks change them if they want it to do something else?

There are two directions we could ultimately go if we want to get more complicated:

codebykat commented 9 years ago

(And just to avoid confusion here, upgrading Horde is a red herring. I don't know what problem @wubbahed fixed by doing so -- possibly something else going wrong with attachments.)

nikolov-tmw commented 9 years ago

Hey @codebykat, glad you've found some time for the plugin :smiley:

I guess using the image_default_size and image_default_link_type would be a good choice for now. I think the only issue with it(if I remember correctly) is that it's set when you insert an image to your post's content, so you can change your settings on accident(?). Or is this not how that setting works?

Ultimately, I think we can get by with settings for image size(dropdown with all image sizes) and whether to wrap the image in a link to the full-size image. Ideally, the gallery shortcode can be reworked to only include images that were not inlined, but that would get a bit more complicated :)

Upgrading Horde for me at least was quite a pain and if the plugin works with the currently used version, then we might as well stick to it(unless there are security issues in the current release of Horde).

nikolov-tmw commented 9 years ago

@codebykat, would you like me to create a merge request for the inline image cid replacement? It would need to get tested very thoroughly though(as in from different mail clients), because I've had it not work at least once so far. Once it broke, because it seems like Apple mail clients like to add 3D after the equals sign of attributes(for instance src=3D"cid:...") - no idea why, but it's pretty silly :)

Should we look into PHP's DOM parsing abilities, instead of relying on regular expressions? Not sure how that would work, or whether it would properly retrieve the value of attributes that are weirdly constructed.

codebykat commented 9 years ago

Or is this not how that setting works?

When you modify those settings after uploading an image, it's just for that image. If you want to change the defaults, you have to use functions.php or change the setting on the full options.php page. (Edit: Unless something has changed since the last time I looked at this, which was awhile ago.)

would you like me to create a merge request for the inline image cid replacement?

Would love a PR! This would be awesome to include in the v1.1 release. :heart:

Upgrading Horde for me at least was quite a pain and if the plugin works with the currently used version, then we might as well stick to it

Horde is a can of worms as far as PHP versions go. I had originally hacked it in (like manually back-ported it to support PHP 5.2) and it's causing no end of trouble. I need to do a major refactor, either removing it entirely or allow for falling back to PHP's IMAP bindings. That's probably the next thing I'm working on.

As for testing, I'm working on the phpunit tests today in hopes of having something more robust in place before I embark on the aforementioned refactor. There are a lot of bugs with different mail clients and they're pretty much impossible to track down right now. I'd recommend saving examples of different emails so that we can run the tests against them and make sure they get parsed correctly.

codebykat commented 9 years ago

Oh, I don't know anything about PHP's DOM parsing, but regex is pretty hard to maintain, so maybe? At least experiment with it and see if it's terrible.

nikolov-tmw commented 9 years ago

When you modify those settings after uploading an image, it's just for that image.

What I just found was that it seems like the size setting(at least) is stored in a user option, so you can generally set it on a global scale, but it also gets overriden on a per-user base.

Would love a PR! This would be awesome to include in the v1.1 release. :heart:

Ok, I'll work on that this coming week and will create the PR :) I'll give DOM parsing a shot at that as well.

codebykat commented 9 years ago

@nikolov-tmw Heads up! I just refactored a bunch of stuff, you'll want to merge w/upstream. (Sorry, it had to happen tho :/ )

nikolov-tmw commented 9 years ago

Hey, no worries :) I've been pretty busy, hence the no PR. I'll refresh my code