I've written some major updates for the image_caption_filter. Attached is a patch. Besides some code cleanup, this one makes the following changes:

  • Allows image width to be specified via the width attribute or an inline style. CKeditor is using inline styles these days.
  • Only operates on images that have a width specified.
  • Allows the user to specify on which image classes it should operate (config page).
  • Uses spans instead of divs. Spans are valid inside p elements but divs are not. The negative side is that you have to format your span to look like a div, otherwise it will look silly.

I'm not an expert on the patch command but I did my best - ICF is the module patch. Attached is a custom README to put into the contrib folder and a patch for the current README to refer to it. Also attached is some doco, intended to replace what is currently on the module page, that I hope will clear up any confusion. You'll probably have to format it slightly to make the code excerpts appear correctly on the page etc, it's hard to do that from here.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bryancasler’s picture

subscribe

bryancasler’s picture

I had to use this "patch" to get the current release of Image Caption to work. Not sure if your patch will resolve this issue with WYSIWYG editors, but I just wanted to point it out.

http://drupal.org/node/368459#comment-1263415

divbox’s picture

Thanks for this updated patch. Works well!!

I am wondering if I am able to access node/cck/form data inside the filter run? Here's what I'm trying to do.

I am using imagefield_extended module to add some additional fields to image cck field. They are then tied to the image node data. I want to use these additional values inside the caption box.

Is this possible, or am I only limited to what is on the img tag inside the filter?

Thanks!
divbox

brian_c’s picture

Inline styles and the width attribute aren't the only way to specify an image's width, it can come from a stylesheet.

For instance in our setup we are using the Insert module, which inserts the image with an imagecache preset class, which we then use for styling. This new patch breaks Image Caption Filter functionality entirely for us because of this, so we had to revert to the old version (which we've hacked to work with our imagecache classes).

Perhaps I'm missing something, but why does Image Caption Filter have to care about the width at all? Shouldn't it simply convert this:

<img src="..." class="{recognized_class}" title="caption text">

to this?

<div class="{recognized_class}">
  <img src="..." title="caption text">
  <div class="caption">caption text</div>
</div>

Why does Image Caption Filter have to concern itself with width at all? We can control image dimensions entirely from CSS just fine with the above markup.

At the very least, I would suggest making this new "enforce width attribute/style" functionality optional according to an admin setting.

fietserwin’s picture

The filter should concern itself with the width, as you probably want the caption be restricted to the width of the image as well. However to get more consistency in a site, it may well be that:
- you size all images/thumbnails within nodes/blocks to a given size using (external) CSS
- or assure that the images are resized tot the correct dimensions on upload, and you thus don't have to use any width style or attribute at all.

So the filter should indeed allow for the case in which no width info is present. To correctly size the caption in the absence of any width info, multiple classes may be needed. So these 2 problems are closely related

Please find attached a patch (and complete resulting code file) that:
- allows for multiple classes on the img tag
- allows for the absence of any width specification on the img tag
- prevents "Undefined offset: 1" notices. Without a match in preg_match(), $matches will be an empty array. Not your fault, the PHP documentation almost consistently fails to specify behaviour and return values in edge cases :(

The code has been tested with these cases:
- no width => no width style on outer span
- width as style (that's what CKEditor emits and what I use) => style declaration defining width on outer span
- multiple classes including one of the triggering classes => filter does its job
- multiple classes not including one of the triggering classes => filter skips the img tag and leaves it unchanged
- no title (with a triggering class) => filter skips the img tag and leaves it unchanged

The patch is based on top of the dev download + the patch provided in the topic start. Perhaps we should make one new patch file that executes both patches on the dev version?

EDIT: I just saw that I forgot to change the help text in the admin interface. It still says a width is needed. No show stopper so I won't change the patch just for that.

Zach Harkey’s picture

Category: task » bug
Status: Active » Reviewed & tested by the community

+1 This patch includes many improvements to the module. I have tested this and it works great.

I'm also re-classifying this issue as a 'bug report' for the following reason:

In HTML the <img> tag is displayed inline by default. So almost every rich text editor out there, (fckeditor, ckeditor, tiny, etc.) will wrap them in <p></p> elements. The current method of using <div></div> elements causes invalid markup and/or extra empty elements depending on the editor.

On the other hand, using <span></span> is completely bullet proof. They can be nested inside of any kind of block or inline element without breaking validation. They can also nest inside a link which makes them work nicely with the <a href=""></a> created by the automatic linking features of the image_resize_filter module.

dkingofpa’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +filter, +width, +float, +caption
FileSize
209.35 KB
230.32 KB
5.57 KB

I'm unable to apply the patch from #5. And using the file image_caption_filter.module.txt from #5 just showed that every line changed (it’s the line endings). But I agree, we need to handle ckeditor setting an inline width so I tried to pick out the changes and manually apply them.

The configuration of the filter probably shouldn’t be under the Site configuration menu. It should be in the Input Format filter configuration form like the other filters.

Attached is a patch rolled against 6.x-2.x-dev that includes the OP's patch as well as the changes from #5. Along the same lines as the inline width being added to the containing span tag, I did the same thing for the float property. CKeditor likes to inline that as well. I attached two images showing how the caption won't line up correctly if we don't add the float property to the containing span tag as well.

For future reference:

dkingofpa’s picture

And in case you don't like to run a dev version of this module on a production install...
here's a version of the patch in #7 but rolled against 6.x-2.5

dsayswhat’s picture

Status: Needs review » Reviewed & tested by the community

I have successfully used the patch from #7 against 6.x-2.x-dev. Thanks to all of you for your hard work.

Concerning #5 above, I implemented this new version on a site with existing images that were already captioned using the image caption module. In trying to get things to work for them, I noticed that some images had widths and some didn't.

In order to ensure that things worked properly I did a little research and found the image resize filter, which puts height and width on images when users forget ( which is all the time). A great help, and makes things behave better without end-user intervention. You can find it at http://drupal.org/project/image_resize_filter.

Beyond that, I noticed some things about the image caption filter that didn't really match my needs - I ended up having to modify it some, so I've opened a new issue to describe the what and why - see #1004454: jQuery and input filter versions of image caption module should match more closely..

Thanks again for your work on this - my clients are happy.

dsayswhat’s picture

In order to get this out where you can see it, (cause there's some more specific modifications that I don't want to clutter the thread with), here's what I spotted while working with the patch:

<span class="image-caption-container">
  <img class="caption" ...>
  <span class="image-caption">caption text here</span>
</span>

The above is more in keeping with the behavior of the image caption module, and might make it easier to drop the filter module in as a replacement for the jQuery version...changing the class usages might be worthwhile. Thoughts?

NPC’s picture

Nice update, thank you! Subscribing in case you decide to change the class-naming conventions, as proposed in #10.

mastfish’s picture

Slight problem with the patch in #8
At line 107(
$classes = preg_split('/\h+/', $class, null, PREG_SPLIT_NO_EMPTY);
)
was matching the character "h", meaning this would always fail on the "image-right" class
Replacing line 107 with:
$classes = preg_split('/\s+/', $class, null, PREG_SPLIT_NO_EMPTY);
fixes the issue for me

fietserwin’s picture

\h is horizontal white space unlike \s which is any white space. But: this is only since PHP 5.2.4, thus that would make this module require a higher version than Drupal 6 core. So, \s is indeed better.

@mastfish: time to upgrade your PHP install? :)

willow315’s picture

I cannot believe that you've created a patch that would break the functionality of the current module. As Brian C said,

"For instance in our setup we are using the Insert module, which inserts the image with an imagecache preset class, which we then use for styling. This new patch breaks Image Caption Filter functionality entirely for us because of this, so we had to revert to the old version (which we've hacked to work with our imagecache classes).

We, too, are using the Insert module, and have set up our images sizes through imagecache.

So, now I cannot update the image caption module at all. Bummer.

In addtion, the module malfuctions when there are quotes or other special characters used in the caption. What's up with that? Is there any solution to that with the module, prior to this patch? I MUST get this quote issue resolved. I don't have time to try to find another way to get captions working on this site.

Thanks, WCW

NPC’s picture

The patch from #8 works for me with Insert and ImageCache. Though it does have issues when there is a quote is in the caption, agreed.

fietserwin’s picture

Special characters in the title text should be converted to their proper html entity. E.g. using wysiwyg + ckeditor, double quotes in the title are properly inserted as " in the text field, as they should be.

So I don't get the situation when this gets wrong as this filter may expect to operate on proper html. Or am I missing something?