Hi,

Is the intention of this module to always over-ride the title field when it is enabled? If so why is there a setting under the insert form item called "Use the Title field as a caption"? When I uncheck this the caption_filter module still over-rides the title field because caption_from_title is never checked.

In caption_filter_element_process() the over-ride is happening when $element['title'] is set. In my mind the line

if (isset($element['title'])) {

should be replaced with

if (isset($element['title']) && isset($widget_settings['caption_from_title']) ? $widget_settings['caption_from_title'] : FALSE) {

Otherwise that setting doesn't seem to be used.

CommentFileSizeAuthor
#8 1985458-caption-filter-respect-title-caption-checkbox-8.patch2.12 KBpopulist
#5 1985458-caption-filter-respect-title-caption-checkbox-5.patch2.05 KBpopulist
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sarenc’s picture

Sarenc CreditAttribution: Sarenc commented

I see that breaks the js but why is the JS being added on hook_init() and not in caption_filter_element_process()?

I suggest removing caption-filter.js from being loaded in hook_init() and instead place it in caption_filter_element_process() as below. I also think "caption_from_title" should be in the title setting of the form not insert. Is the caption functionality supposed to be forced on people who don't have insert installed?

/**
 * Form API #process function for elements.
 */
function caption_filter_element_process($element) {
  static $js_added;

  // Bail out early if the needed properties aren't available. This happens
  // most frequently when editing a field configuration.
  if (!isset($element['#entity_type'])) {
    return $element;
  }

  $field = field_info_field($element['#field_name']);
  $instance = field_info_instance($element['#entity_type'], $element['#field_name'], $element['#bundle']);

  $widget_settings = $instance['widget']['settings'];
  $widget_type = $instance['widget']['type'];
  
  //if (isset($element['title'])) {
  if (isset($element['title']) && isset($widget_settings['caption_from_title']) ? $widget_settings['caption_from_title'] : FALSE) {
    $element['title']['#title'] = t('Caption');
    $element['title']['#description'] = NULL;

    // Add settings for this widget only once.
    if (!isset($js_added[$widget_type])) {
      $js_added[$widget_type] = TRUE;
      $caption_settings = array(
        'captionFromTitle' => $widget_settings['caption_from_title'],
      );
      drupal_add_js(drupal_get_path('module', 'caption_filter') .'/js/caption-filter.js', array('every_page' => TRUE));
      drupal_add_js(array('captionFilter' => array('widgets' => array($widget_type => $caption_settings))), 'setting');
    }
  }

  return $element;
}

/**
 * Implements hook_init().
 */
function caption_filter_init() {
  $path = drupal_get_path('module', 'caption_filter');
  drupal_add_css($path .'/caption-filter.css', array('every_page' => TRUE));
}

Let me know if you'd like a patch with these changes.

kmonty’s picture

kmonty
Location San Francisco, CA
CreditAttribution: kmonty commented

Are you sure you are using the latest -dev? We already removed the js out of hook_init in a previous commit. See: #1898986: add js through hook_page_build() and not hook_init()

Also, please create separate tickets for different issues, as it helps everyone focus on a single problem at a time. Thanks!!

Sarenc’s picture

Sarenc CreditAttribution: Sarenc commented

Hey kmonty,

You were right I wasn't using dev.

Dev does seem to have the same issues though. If I un-check "Use the Title field as a caption" there is no difference. The widget is still altered to change "Title" to "Caption" and when inserted the image still gets wrapped in the caption code. So it appears $widget_settings['caption_from_title'] is un-used. I tried the modification I posted in the first post and got JS errors that left insert not working. The only way I could get the functionality of the caption filter without altering any form items was deleting most of the functions in the module.

Olivier Beerens’s picture

Olivier Beerens CreditAttribution: Olivier Beerens commented
Version: 7.x-1.x-dev » 7.x-1.2

Got the same problem here : I don't want to use this feature because it's breaking in Wysiwyg (but i have to leave the module actived because Panopoly use it), so i leave the checkbox "Use the Title field as a caption " uncheck but it's still put caption bracket in the wysiwyg field with insert module...

populist’s picture

populist CreditAttribution: populist commented
Category: support » bug
Status: Active » Needs review
FileSize
1985458-caption-filter-respect-title-caption-checkbox-5.patch2.05 KB

I checked out this issue and can see the same thing reported above using latest version of caption filter, insert, and panopoly. Things work OK when you enable "Use Title Field as Caption" but when you disable it the functionality remains.

The solution direction proposed in #1 is the right one I think, but it requires some additional conditional logic in the JS since the drupal_add_js() call does need to be made to activate the caption filter magic in the first place (even if the use title as caption option is 0).

The attached patch, I believe, solves the issue by allowing you to use caption filter but have the checkbox for use title as caption be respected.

populist’s picture

populist CreditAttribution: populist commented

As a followup, to test the patch in #5 you will need to enable insert.module and add an image to a content type. In the field settings you can set or unset the "Use Title as Caption" option which should work in *both* cases when you send the image to a WYSIWYG.

A few caveats:

-- the patch still uses caption filter to add the image, it just doesn't add the title as caption. not sure *exactly* what the interaction is intended between insert.module and caption_filter.module, but this is one way to solve it. the other one is to just ignore caption filter entirely but that should require some sort of additional help text since its a bit confusing.

-- not sure what happens if you have two insert images on one form which both have a handler like image_image and one has the option checked to use title as caption and the other doesn't. i think drupal will pick the first one and make that be the config for both, but didnt test that very much.

Sarenc’s picture

Sarenc CreditAttribution: Sarenc commented

Thanks for the patch. I tried it out and noticed the image field wasn't being altered after turning off "Use Title as Caption" which is great.

Unfortunately the caption tags were still added to the wysiwyg although they were empty. A different problem was the image was being inserted into the wrong text area.

populist’s picture

populist CreditAttribution: populist commented
FileSize
1985458-caption-filter-respect-title-caption-checkbox-8.patch2.12 KB

Here is a re-roll of the patch from #5 against the latest -dev:

Arnaud Vromman’s picture

Arnaud Vromman CreditAttribution: Arnaud Vromman commented

Hi,
I was having the #1303026: Undefined index: caption_from_title issue on D7.

I tried your patch and it didn't solve the issue so i checked the code :

-  if (isset($element['title'])) {
+  if (isset($element['title']) && isset($widget_settings['caption_from_title']) ? $widget_settings['caption_from_title'] : FALSE) {
     $element['title']['#title'] = t('Caption');
     $element['title']['#description'] = NULL;
+  }                                                   <---------------- this bracket
 
-    // Add settings for this widget only once.
-    if (!isset($js_added[$widget_type])) {
-      $js_added[$widget_type] = TRUE;
-      $caption_settings = array(
-        'captionFromTitle' => $widget_settings['caption_from_title'],
-      );
-      drupal_add_js(array('captionFilter' => array('widgets' => array($widget_type => $caption_settings))), 'setting');
-    }                                   <---------------------- the other one

Then I asked myself : why did you add this bracket right before the comment line and remove the last one ?
(it was obvious because the indentation was modified)
I put it right where it was before and i never saw the error again.