Given that the module already implements a TinyMCE plugin to render the caption tags, I think it would be handy if it also provided a button to insert/change the caption tags, without the user having to get into the actual code. So I went ahead and wrote one. :)

The patch file and an image to use for the button are attached for your review... Just apply the patch as normal, and then copy the image into the module's js folder. You'll also have to clear your cache after applying.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mixologic’s picture

didnt work for me. But then I realized I was being barbaric and trying to run patch -p0 << caption-button.patch.

git apply caption-button.patch works great for me. This is pleasing. Thanks

(note that I did not mark rtbc as I dont know that copying the langs/en.js stuff into this folder is the right way to go, I know it'll not work with any other language.. plus you're adding third party code outside of the libraries folder. So something needs to be tweaked to not make that necessary)

muriqui’s picture

Re: langs/en.js, that's not third party code, and wasn't copied from anywhere. Rather, it's code written to properly implement a TinyMCE button plugin, just like the code in caption-filter-tinymce-button.js... Thus, it belongs with the module implementation. It shouldn't (and couldn't) be in sites/all/libraries, because it's not a library, and because TinyMCE wouldn't look for it there, so the button would have no description at all.

I do agree that it would be better if we could use Drupal's native internationalization, but that's not how TinyMCE works. It has it's own internationalization method, and expects a langs folder to exist in the plugin with localized .js files per language. So since we're providing a TinyMCE plugin with this module, it unfortunately has to do it by TinyMCE's rules. Other Drupal modules that include TinyMCE plugins do the same.

Mixologic’s picture

Got it, I saw langs/en.js inside of libraries/tinymce/jscripts/tiny_mce/langs and thought it was the same file for some reason (as my original attempt at patching didnt apply so it couldnt find that file).

I did run into some issues with this patch after all.

I needed the caption text itself to be styled, and since this creates a <div><img>raw text</div> there wasnt any way to target the text without also affecting the image. I added a <p class="caption-text">raw text</p> into the patch along with the regular expressions.

Additionally It didn't seem to work very well with editing an existing caption - it would replace the text, but not the original [caption] tags and img tag. So it would end up with a duplicate image and extra caption box.
It seemed like there was some issue with "ed.execCommand('mceInsertContent', false, tag)" - Im not sure if that was because the tag variable was reused to be the replacement text. Some poking around in the tinyMCE forums seemed to indicate that other folks were having problems with that method as well, so I adjusted it to use ed.dom.remove(p[1], false) to completely remove the original content if we are editing an existing caption.

Attached is the original patch, plus my modifications.

jessehs’s picture

This patch works well for me.

One gotcha is that the line break converter filter must either be turned off, or moved so that it runs after the caption filter.

I've included a re-rolled version with a fix for using it with the admin_menu module, as the admin menu appears in the popup.

budalokko’s picture

Patch #4 worked for me too.

populist’s picture

Status: Needs review » Needs work
FileSize
9.64 KB

Here is a re-roll of the patch from #4 which includes the image binary file. I did a pretty thorough review of things and here are a few issues I still see outstanding:

1.) Inconsistent experience when doing replacement of the caption text/float. Often the image gets replaced with an "undefined" which is a little wierd.

2.) Resizing method for the caption wrapper doesn't work when the image is initially added and requires editing/saving to become functionality

3.) There is a problem with calculating the widths when max-width: is set on the image AND it has a width="" property which creates a wierd user experience.

4.) It would be nice if the "caption" text entry box was a little larger since only one line is somwhat limiting for extended captions (i.e. description of image and credits).

populist’s picture

Status: Needs work » Needs review
FileSize
10.47 KB

Alright. Here is an even better patch that addresses my concerns before. In this patch:

1.) The inconsistent experience with replacement was due to a misconfiguration (on my end) of WYSIWYG Filter. It requires that P tags have classes.

2.) Resizing method for the caption wrapper was made to work by removing the specific exception for chrome/safari/opera since TinyMCE now supports it natively

3.) The problem with max-width was corrected by adding a max-width to the CSS for caption-inner

4.) The caption text entry box was made a textarea and now has 4 lines. I also cleaned up the float option for no float by just saying "None".

sylus’s picture

Duplicate Post

sylus’s picture

For people using an i18n setup and adding an additional language to TinyMCE the Wysiwyg is broken if the additional language file is not present. In this case langs/fr.js

This patch adds that file as well so at least english + french i18n setups are working.

quicksketch’s picture

Thanks guys. I'm not keeping a very close eye on this queue as it doesn't show up in my list of projects (I'm not the lead maintainer). I'll review when I get the chance.

sylus’s picture

Hmm is it just me or is the gif not included in any of these patches?

beeradb’s picture

Here's a re-roll of #9 which includes the image

sylus’s picture

Thanks @beeradb!

quadcomm’s picture

Version: 7.x-1.x-dev » 7.x-1.2
Status: Needs review » Needs work

I understand this discussion is about 7.x-1.x-dev, however I applied this patch to 7.x-1.2. The [caption] tag is not always parsed in HTML.

I tried changing the Filter Processing Order to no avail.

I then removed <p class="caption-text"></p> from js/caption-filter-tinymce-button.js and now everything seems to work fine so far.

The following example is when [caption] won't get parsed:
[caption]<img src="" /><p class="caption-text">This is a caption</p>[/caption]

The following example is when [caption] will get parsed:
[caption]<img src="" />This is a caption[/caption]

Why do we need <p class="caption-text"></p> anyway? The output looks the same without it.

Mixologic’s picture

Version: 7.x-1.2 » 7.x-1.x-dev
Status: Needs work » Needs review

The <p class="caption-text"></p> is needed so that the caption text can be targeted with css and javascript. Bare text inside of a div is hard to do anything with, and additionally has no semantic value. As an example, imagine a screen reader scanning through the content and encountering bare text immediately after an image tag. The reader would have any no way of knowing if that were a caption or not, and whether or not to read it aloud.

Also, applying a patch that needs review to an older version of code (1.2) is not intended to work, as the patch may interact with other patches already committed to dev. If you'd like to help out by reviewing the patch, its most helpful to apply it to the latest development version and see if it works or fails for you in your environment.

dawansv’s picture

OK, so I first tried #12 on a fresh dev install twice and it did not work (no button shows up). I did it again with patch #7 and it worked. I have not tried #8 and #9 but will.

Also, I understand the need for <p class="caption-text"></p> for this to work, but it means that caption-filter.js also needs to be patched for the p tag to be added when a [caption] entry is being created via the insert module, otherwise the feature cannot be used to edit an existing caption created that way.

For that to happen Line 11 of caption-filter.js needs to be:

    options['content'] = '[caption]' + options['content'] + '<p class="caption-text">' + options['fields']['title'] + '</p>' + '[/caption]';

(sorry I don't have what is needed to create a patch)

jessehs’s picture

This is a re-roll of #12, but removes the "width: 100%" change to the CSS file, because that change was committed separately, and is no longer needed.

This patch also fixes a few whitespace errors that were reported during the "git apply" command.

It also adds the change to caption-filter.js requested in #16. Please test this change, as I have not. I have only confirmed that this patch applies cleanly against HEAD.

kmonty’s picture

Just a friendly heads up: the <p class="caption-text"></p> is a separate issue and won't be considered as part of TinyMCE integration. (But feel free to open new feature request for it!)

populist’s picture

Here is an updated patch that hopefully contains all the things that need to be done. It rolls out the previously coded TinyMCE WYSIWYG button integration, but also includes a few more things:

  • Caption Attribute - This patch moves the storage of the "caption" text into the [caption caption="VALUE"] which is consistent with the previous WordPress standard (http://justintadlock.com/archives/2011/07/01/captions-in-wordpress) as well as the existing logic in _caption_filter_replace().
  • Double Quotes Encoding - This patch will escape double quotes which might be part of a caption (i.e. Dries "Drupal Master" Buytaert). This is necessary to support the Caption Attribute magic.
  • "Caption-Text" Class - This patch wraps the displayed caption in a P tag with the class "caption-text" which will be helpful to themers in targeting the caption text for various front end requirements as well as being important (as per #15 and #16) for implementing the WYSIWYG integration. We really need a clean way to target the caption data.
  • Standard Markup - This patch does away with the previous approach of saving the caption-text class to the body field in favor of the Caption Attribute approach. Instead, it adds the appropriate markup in _caption_filter_replace() which creates consistency between the WYSIWYG look and the final output.
  • Documentation - This patch updates the documentation to talk about the new way of storing caption information in both the README and the inline help.
  • Backwards Compatibility - This patch does *not* modify the markup or experience for folks who are not using WYSIWYG or prefer to keep the existing caption style.
  • Healing for Folks Who Used Previous Patches - This patch also provides some healing for folks who have captions from the previous patch. Basically, the WYSIWYG plugin will first look for caption information as part of the attribute, but failing that it will look for the previously specified P tag and will move that to caption attribute if the caption is loaded in the WYSIWYG.
populist’s picture

Quite update to #19 to provide Double Quote Encoding for editing the captions in the WYSIWYG.

populist’s picture

Additional update to #20 based on feedback:

Backwards Compatibility - This patch does *not* modify the markup or experience for folks who are not using WYSIWYG or prefer to keep the existing caption style.

Backwards Compatibility - This patch will modify the markup for existing captions by adding the P tag with the caption-text class. This helps to provide consistency with existing users who are using this patch already and have this markup (25% of users) and consistency in general for themers. CSS has been modified to target that P tag to prevent unexpected margin on current sites.

populist’s picture

Here is a version that applies against -dev:

populist’s picture

Here is a version with the .gif --binary style:

populist’s picture

And a final patch to adjust a few more JS/CSS things. Looking really sharp!!

kmonty’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

fixed missing word

belong@mywebworkshop.com’s picture

Issue summary: View changes

This does not appear to be part of the actual module. I have the module installed, enabled in TinyMCE, but no button in TinyMCE. Is it going to added in to the module?