In the alpha3 release, Flexslider moved preprocess for flexslider_list to a process function: http://drupalcode.org/project/flexslider.git/blobdiff/dc3884f4e34eb0a644...

This now causes template_preprocess_flexslider_picture_list() to throw a fatal error since it manually calls template_preprocess_flexslider_list(). I'm not sure if the fix is as simple as using template_process_flexslider_list() instead.

Comments

jwilde’s picture

Nice catch. Changing it to, template_process_flexslider_list fixed it for me. JIm

abdullahgz’s picture

Is there a patch for this issue

drupalgideon’s picture

Not sure if the same thing, but I get the following error after updating to alpha 3

Notice: Undefined index: uri in theme_flexslider_picture_list() (line 56 of /sites/all/modules/contrib/picture/flexslider_picture/theme/flexslider_picture.theme.inc).

The $vars['items'] array passed to the theme function in alpha 1 has a 'thumb' element in the array (full path to thumbnail image), but has 'caption' in alpha 3 (in my case this is NULL).

jantoine’s picture

Status: Active » Needs review
StatusFileSize
new1.96 KB

The attached patch updates the template_preprocess_flexslider_picture_list() function to be template_process_flexslider_picture_list() (a process function instead of a preprocess function) to match the flexslider module. It also fixes the calls to the updated functions in the flexslider module.

chris burge’s picture

Thank you for writing this patch. My slider works again; however, I now get the following notices:

  • Notice: Undefined index: uri in template_process_flexslider_picture_list() (line 79 of /var/www/example.com/sites/all/modules/contrib/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
  • Notice: Undefined index: uri in template_process_flexslider_picture_list() (line 79 of /var/www/example.com/sites/all/modules/contrib/picture/flexslider_picture/theme/flexslider_picture.theme.inc).

I don't know if what I am describing is a symptom of this particular issue or another issue that hasn't been reported yet.

Set up Info

Using Views to create slider:

  • Block display (also have tried Page display)
  • 'Flexslider' style format
  • 'Picture' image formatter

Codebase

  • Drupal 7.23
  • Breakpoints 7.x-1.1
  • Flexslider 7.x-2.0-alpha3
  • Picture 7.x-1.1+48-dev (patched with #4)
  • Views 7.x-3.7
dieuwe’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the patch, it works for me. Don't see any errors like #5 right now, but I will post back here if I do.

dieuwe’s picture

Status: Reviewed & tested by the community » Needs review

I enabled the flexslider thumbnail paging controls and now I get the same messages as #5.

dieuwe’s picture

It seems to me that flexslider has changed the way it handles thumbnails. I wouldn't know where to start looking though.

gilsbert’s picture

The patch #4 worked for me too.

May it be commited?

attiks’s picture

Status: Needs review » Fixed
darren oh’s picture

Fixed in commit 20473c8.

das-peter’s picture

Issue summary: View changes
Status: Fixed » Needs review
StatusFileSize
new4.24 KB

Just wondering, has anyone got responsive images with this patch? All I seem to get are the normal flexslider images - not the ones from picture.
To make that happen I had to patch the module as seen in the attached patch.
For me it looks like not only the method name changed but also the return parameter - which needs different handling now.
Attention: This changes make (including the already applied patch) make picture flexslider incompatible with flexslider versions prior alpha 3. I'm fine if that's intended, but I think it should be documented.

alexander.sibert’s picture

I have the same issue:

Setup:

  • Current Picture Dev Release
  • Breakpoints 1.1 Stable Release
  • Flexslider 2.0 Alpha-3 Release

I use Panels 3, Views with Flexslider, Picture, Breakpoints. Here you see http://lafox.de - the Flexslider works but i receive errors on top.

Notice: Undefined index: uri in template_process_flexslider_picture_list() (Zeile 79 von sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
Notice: Undefined index: uri in template_process_flexslider_picture_list() (Zeile 79 von sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
Notice: Undefined index: uri in template_process_flexslider_picture_list() (Zeile 79 von sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
Notice: Undefined index: uri in template_process_flexslider_picture_list() (Zeile 79 von sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).

Update

I updated now Picture/Flexslider and the Flexslider Library to current Dev releases but the errors are not away.

dieuwe’s picture

das-peter: No, responsive images were not fixed in the patch, as you will see from comment #5 in particular.

I'll try the patch in #12 sometime next week, I'm just too busy right now. I too would be fine with breaking everything prior to flexslider alpha 3 as this is this module obviously needs to keep up.

dieuwe’s picture

Current dev code with flexslider thumbnail paging enabled:

Notice: Undefined index: uri in template_process_flexslider_picture_list() (line 79 of /var/www/example/sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
Warning: Illegal string offset 'thumb' in theme_flexslider_picture_list() (line 102 of /var/www/example/sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).

Patched from comment #12:

Notice: Undefined index: item in template_process_flexslider_picture_list() (line 54 of /var/www/example/sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
Notice: Undefined index: item in template_process_flexslider_picture_list() (line 55 of /var/www/example/sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
Notice: Undefined index: item in template_process_flexslider_picture_list() (line 56 of /var/www/example/sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
Notice: Undefined index: item in template_process_flexslider_picture_list() (line 57 of /var/www/example/sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
Notice: Undefined index: item in template_process_flexslider_picture_list() (line 58 of /var/www/example/sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
Notice: Undefined index: uri in template_process_flexslider_picture_list() (line 75 of /var/www/example/sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
Warning: Illegal string offset 'thumb' in theme_flexslider_picture_list() (line 98 of /var/www/example/sites/all/modules/picture/flexslider_picture/theme/flexslider_picture.theme.inc).
das-peter’s picture

Updated the patch to fix the integration into flexslider >= alpha3.
Changes:

  • Fixed optionset form alteration - proper integration into the new vertical tabs (Related issue #2149917: FlexSlider - Cannot add/modify optionset - SQL error).
    In the latest flexslider version the image style is set in the field formatter and not in the optionset itself. We should do the same thing and move the selection of the mapping to the field formatter settings in a next step.
  • Fixed call to theme_flexslider_list_item - set the proper settings array.
  • Fixed flexslider_picture_field_formatter_view to avoid processing not available data.

It looks like quite some parts of the flexslider code was changed. And as far as I can see it looks much cleaner now.
The option set now only defines how the flexslider behaves and not which image style is used. The image style is defined in the field formatter settings now.
This is promising because I think that way we can provide our own field formatter with the settings to define the flexslider optionset and the picture group to use. This would loose the coupling and make it easier to keep up with flexslider overall.

das-peter’s picture

Attention: This patch will break your existing configuration!

The attached patch contains the big change I proposed in #16:
The configuration of the picture group / colorbox stuff doesn't reside anymore in the flexslider optionset but in the field settings of our own field formatter.
Instead hijacking theme functions we now have our own field formatter and use it (flexslider_picture_field_formatter_view()) to prepare the items to pass them to flexslider theming.
Advances of that are:

  • At least a bit a better decoupling between the two modules
  • Less hijacking / alteration of existing code / behavior
  • Overall less code

Attention: This patch will break your existing configuration!

rcodina’s picture

The patch on #17 works for me using this versions:

-Drupal 7.24
-Breakpoints 7.x-1.1
-Picture 7.x-1.2 with #17 patch
-Flexslider 7.x-2.0-alpha3
-jQuery FlexSlider v2.2.0

Also, in manage display I had to select "Picture" formatter instead of "Flexslider Picture" formater to get responsive Flexslider to work as expected.

The "Flexslider Picture" formatter does not work: additional blank slides are added and loaded slides turn to blank soon after loading.

Thank you das-peter!!!

robcarr’s picture

Patch at #17 applied cleanly for me, and has removed numerous errors (also including #2072515: Illegal string offset 'thumb' in theme_flexslider_picture_list()).

This is the first project I've used Picture and Flexslider, and the settings now seem a little easier to use from an 'incomers' perspective. This patch would get my support, but it would be good to get a slightly wider review before setting to RTBC, as I'm not sure of any wider implications to such a substantial change to the module code.

FranCarstens’s picture

I have the same issue as "tmtheowner". Thumbnail images do not show up at all with the patch in #17 using "Flexslider Picture" formatter. My setup is exactly the same as post #18

URL display as: sites/default/files/styles//public/...

It looks like the thumbnail style isn't called.

== EDIT

Not sure if this is new, but it looks like Flexslider does not use a thumbnail image style anymore. It merely resizes the slides, thereby loading only 1 set of images. Pros and cons I guess; but this makes a module like flexslider_picture even more valuable.

hydra’s picture

In a simple flexslider configuration without thubnail configuration and picturefill method the patch in #17 works for me.

Ravenight’s picture

I fixed this in a very hacky way but it works for now till a better solution comes along.

Using:
-Flexslider - 7.x-2.x-dev (updated as of this date)
-Picture 7.x-1.2 with #17 patch
-jQuery FlexSlider v2.2.0

In flexslider_picture.module I added the following on line 216:

      // Now render the configured slide to ensure it works with the flexslider.
      $item['slide'] = render($item['slide']);

      // FIX MISSING THUMBNAILS WITH TEMP FIX - String replace
      $new_array = explode('<span',$item['slide']);   
      // get the bad URL
      $new_src = trim($new_array[2]);
      $new_src = substr($new_src, 10);
      $right_src = substr($new_src, -45);
      $new_src = str_replace($right_src,'',$new_src);
      $bad_url = trim(str_replace('"','',$new_src));
      // get a valid URL
      $new_src = trim($new_array[3]);
      $new_src = str_replace('data-media="(min-width: 0px)" data-src="','',$new_src);
      $new_src = str_replace('"  data-width="','     ',$new_src);
      $new_src = str_replace('data-height="','     ',$new_src);
      $new_src = str_replace('"','',$new_src);
      $right_src = substr($new_src, -22);
      $new_src = str_replace($right_src,'',$new_src);
      $good_url = trim($new_src);
      // replace the bad url with the valid URL
      $item['slide'] = str_replace($bad_url,$good_url,$item['slide']);

Its hacky as all get-out but works and let me use picture, colorbox, and flexslider with thumbnails and will allow a thumbnail to be created if the image style does not already exist.

c-c-m’s picture

Status: Needs review » Needs work

I applied the patch in #17 and it worked fine except for the fact that the slider started to display a caption (I had this option disabled) and I didn't find any place in admin UI to change that. If I am not wrong, I think this option used to be available on UI.

indigoxela’s picture

Hi,
patch #17 partly solved my picture/flexslider problems.

My setup:

  • picture 7.x-1.2 (patch #17 applied)
  • breakpoints 7.x-1.1
  • flexslider 7.x-2.0-alpha3
  • flexslider library: v2.2.2

Before patching, error messages appeared (Undefined index: image_style ...) after patching these messages are gone.

After patching - unlike mentioned in #18 - "Flexslider Picture" formatter worked for me as expected. The correct image sizes get loaded.

What does not work: responsive thumbnails

With a normal (not responsive) Flexslider, thumbnails appear, but are only resized via css, not via image style.
With Flexslider Picture, paths to thumbnail image styles are empty like mentioned in #20, so images can't get loaded.

Should we add a "Thumbnail" setting to Flexslider Picture formatter settings?
Or should we grab it from flexslider's option set? The setting is still there, but is not in use anymore (or temporarily not in use?).

EDIT: as a reference - thumbnail discussion on Flex Slider module issue list: https://drupal.org/node/2123947 (Thumbnail image style is not being applied)

indigoxela’s picture

@ c-c-m (comment #23)

I can find the caption setting on the image field's display setting (/admin/structure/types/manage/CONTENTTYPE/display),
right beneath "Picture group".

indigoxela’s picture

Hi,
to come back to the thumbnail problem with patch #17 applied:

Not the thumbnail image_style itself is the problem, but the fallback_image_style.
Fallback images are broken.

Flexslider module grabs the image src via preg_match(). That would actually work even for flexslider_picture thumbnail items (although very un-responsive!), but unfortunately the variable fallback_image_style seems empty at that point.

Fallback images work in picture but not in flexslider_picture.

EDIT: oops, that problem is actually easy to fix...
$item['slide'] array (flexslider_picture.module, around line 193 if patched) is missing the #style_name setting:
...
'#style_name' => $fallback_image_style,
...

indigoxela’s picture

StatusFileSize
new22.65 KB

So, to make testing easier, I created a patch with the changes of patch #17 plus the little modification mentioned in #26 to make thumbnails in flexslider_picture work again.

Still thumbnails are created in the new Flexslider way (simply by grabbing the normal slider image, the fallback image with picture in our case, and resizing it via css). Keep that in mind, if you set your fallback image to original image size.

Kojo Unsui’s picture

I had following problems :

  1. Picture image styles were not loading, only fallback style
  2. Empty thumbnails paths
  3. captions not showing up

With setup:

  • D7.24
  • Picture 7.x-1.2+6-dev
  • Breakpoints 7.x-1.1
  • FlexSlider 7.x-2.0-alpha3
  • Flexslider library: v2.2.2

Applying patch #17 solved 1 : resizing browser now loads corresponding image style, but still no thumbnails
Applying #26 solved 2 : thumbnails appeared (make sure to select a thumbnail style, because the same style is applied whatever windows size...)

So great patch #17, thanks Das-Peter (and Indigoxela for code snippet) ! I think this could be committed to dev ASAP ?

Last thing missing is caption. I have both alt and title enabled for my images, but the formatter returns a disabled checkbox anyway (in panels). In structure/types/manage/.../display the checkbox is not disabled, but it is in Panels... Any idea to fix that ?

And what about optional image, when field is empty and returning default image ? At the moment it returns Notice : Undefined index: width in flexslider_picture_field_formatter_view() (line187 in ... sites\all\modules\picture\flexslider_picture\flexslider_picture.module).

EDIT: patch #27 is certainly ok, but at the moment I tried it I misconfigured sthing, so I reverted it and applied separately #17 and #26 ...

indigoxela’s picture

Hi,
yes, there needs some more work to be done.

@Kojo Unsui: I'm afraid, I can't help with captions and panels. The problem only occurs with panels? I can't reproduce it outside panels.

@das-peter: "Notice : Undefined index: width..." when using a default image in field and there is no user-contributed image.

That could be fixed with, for example:

      // Setup the variables for calling theme_image_style
      if (isset($item['item']['is_default']) &&  $item['item']['is_default'] === TRUE) {
        /* it is a default image */
        $item['item']['width'] = $item['item']['image_dimensions']['width'];  
        $item['item']['height'] = $item['item']['image_dimensions']['height'];
      }
      $item['slide']['path'] = $item['item']['uri'];
      $item['slide']['style_name'] = $settings['image_style'];
      $item['slide']['width'] = $item['item']['width'];
      $item['slide']['height'] = $item['item']['height']; 
      $item['slide']['alt'] = $item['item']['alt'];
      $item['slide']['title'] = $item['item']['title'];

By checking if it is a default image, which has its dimensions set differently (row 184 in flexslider_picture.module)

Would you mind to rework your patch?
Actually I don't understand, why $item['slide'][xxx] has to be set anyway, because right after these assignments the array for theme_image_style is created from scratch and $item['slide'][xxx] isn't used anywhere.
Am I missing something?

A quick test showed, that it all works fine without extra assigning of $item['slide'][xxx].
The only thing we need is to set $item['item']['width'] (and height), for default images.

Kojo Unsui’s picture

Thanks for your code about default image, I'm gonna try it.

The caption problem only occurs with panels. With same content type and same field:

  • in the formatter in structure/types/manage/.../display the checkbox "Use image title as the caption" is not disabled, and the caption is correctly displayed.
  • in Panels, in the formatter styles window the checkbox is disabled.

May I open another issue for that specific point ?

Kojo Unsui’s picture

Right Indigoxela #29 corrects the notices for default image. I only changed $item['item']['image_dimensions'] by $item['item']['is_default'].

So this goes around line 184, after // Setup the variables for calling theme_image_style

<?php 
if (isset($item['item']['is_default']) &&  $item['item']['is_default'] === TRUE) {
  /* it is a default image */
  $item['item']['width'] = $item['item']['is_default']['width'];
  $item['item']['height'] = $item['item']['is_default']['height'];
} ?> 
indigoxela’s picture

Hi Kojo Unsui,
just curious, why did you use $item['item']['is_default']['width']?
It is NULL actually, while $item['item']['image_dimensions']['width'] holds the width of the original image (same for height).
At least this is the case in my test drupal.

Kojo Unsui’s picture

@indigoxela, I used 'is_default' because in this case :
- the field image is optional
- you loaded a default image
- but no other image yet...

$item['item']['is_default']['width'] is fine, while 'image_dimensions' returns undefined.

indigoxela’s picture

Hi Kojo Unsui,
weird.
Still different here, no matter how often I flush cache.

What we are testing: the image field is optional, has a default image and user didn't add an image, so the default image is shown.
I hope at least that is clear.

We are fiddling in function flexslider_picture_field_formatter_view.

Here I have:
var_dump($item['item']['image_dimensions']);
array(2) {
["width"]=>
string(3) "960"
["height"]=>
string(3) "960"
}

var_dump($item['item']['is_default']);
bool(true)
So here a
var_dump($item['item']['is_default']['width']);
is NULL
That doesn't show any error, but image attributes (width/height) for default image are missing.

By the way: $item['item']['image_dimensions'] here always has values, even for images uploaded by users.
It is the array with width and height of the original image (not with any image style applied).

There could be some differences in our testing setups, let's see:
Drupal: 7.26
Picture: 7.x-1.2 (flexslider_picture submodule patched)

Field:
Widget: Multiupload
Required: no, optional field
Default image: yes, is set
Values: unlimited
Filesystem: public, no special subdir
Max dimensions: no limit

Any clue?

EDIT: wait a minute... you are testing it in a panel?
Possibly the original image dimensions get lost within panels.

Kojo Unsui’s picture

Well, I tested both with and without Panels. Yes the test conditions are the same :
image field optional, has a default image and user didn't add an image, so the default image is shown.
Values: unlimited / Filesystem: public, no special subdir / Max dimensions: no limit

Setup slightly different, check #28

We have the same here :
- var_dump($item['item']['is_default']) returns bool(true)
- var_dump($item['item']['is_default']['width']) is NULL
- It doesn't show any error

But var_dump($item['item']['image_dimensions']) is NULL

This is why I altered your code.

prineshazar’s picture

#17 worked for me. Thanks das-peter!

As others have pointed out.

1. Apply the patch
2. Run update.php / drush updb
3. Go to "Manage display" sections and change formatter from flexslider to "Flexslider picture".

indigoxela’s picture

Hi Kojo Unsui,
ahhh, sloppy reading... you already determined that you use Core D7.24.

Another try in flexslider_picture_field_formatter_view:

      // Setup the variables for calling theme_image_style
      if (isset($item['item']['image_dimensions'])) {
        /* user contributed image or default image - original image dimensions */
        $width = $item['item']['image_dimensions']['width'];  
        $height = $item['item']['image_dimensions']['height'];
      } else {
        /* we don't have dimensions for some reason, theme_image_style will catch that */
        $width = NULL; 
        $height = NULL;
      } 
        
      // Replace the normal slide with the responsive one.
      $item['slide'] = array( 
        '#theme' => 'picture',
        '#style_name' => $fallback_image_style,
        '#uri' => $item['item']['uri'],
        '#height' => $height,
        '#width' => $width,
        '#alt' => $item['item']['alt'],
        '#title' => $item['item']['title'],  
        '#breakpoints' => $breakpoint_styles,
      );

This works fine here and doesn't throw any error, even if $item['item']['image_dimensions'] is not set.
This way it is a bit easier, as $item['item']['width'] and height is not needed at all (which is not set for default images).

Kojo Unsui’s picture

I'm not sure which solution is the simplest one, but #37 also is working in my setup . It's up to you.

In the meanwhile, I solved #30 Panels caption checkbox disabled (not related to Picture in fact) altering the Formatter Styles form.

<?php
/**
* Implements hook_form_FORM_ID_alter().
*/
function YOUR_MODULE_form_ctools_entity_field_content_type_formatter_styles_alter(&$form, &$form_state, $form_id) {
  $form['caption']['#disabled'] = FALSE;
}
?>
indigoxela’s picture

Hi,
unfortunately I found another problem: thumbnails are also broken after patch (#17, #27) when using colorbox in field display.

Probably it's easy to fix by (again) fiddling in flexslider_picture_field_formatter_view:

      // If colorbox is enabled build configuration for
      // picture_formatter_colorbox theme.
      if (!empty($settings['colorbox']['enabled'])) {
        $item['slide'] = array(
          '#theme' => 'picture_formatter_colorbox',
          '#item' => $item['item'],
          '#path' => $item['item']['uri'],
          '#image_style' => $fallback_image_style,
          '#breakpoints' => $breakpoint_styles,
          '#colorbox_image_style' => $colorbox_fallback_image_style,
          '#colorbox' => $colorbox_breakpoints,
          '#colorbox_group_id' => $colorbox_group_id,
        );
      }

We should put things together for a new patch (fix for notice : undefined index, fix for colorbox thumbnails...)
Hope to find the time the next few days, but I would be glad if someone else could help.

Anyone willing to test is also very welcome.

indigoxela’s picture

Status: Needs work » Needs review

Hi,
here is a combination of all changes, beginning with the major change introduced in #17 by das-peter.

Also included:
#26: added "#style_name" to bring back thumbnails
#37: prevent php notice problems when using default image
#39: make thumbnails work with colorbox

#17 is a bigger change. If you haven't applied any of the patches yet you have to:

  1. Apply this patch
  2. Run update.php
  3. Go to "Manage display" sections and change formatter from flexslider to "Flexslider picture"
indigoxela’s picture

StatusFileSize
new22.78 KB

OK, and now the patch... ;)
Forgot to attach it in #40.

Ravenight’s picture

#41 worked for me.

rodpal’s picture

#41 worked great! Thanks

  • Commit 20473c8 on 7.x-1.x, picturefill2 authored by jantoine, committed by attiks:
    Issue #2084979 by jantoine | impleri: Fixed flexslider_picture() broken...
attiks’s picture

Status: Needs review » Fixed

  • Commit 20473c8 on 7.x-1.x, picturefill2, 7.x-2.x authored by jantoine, committed by attiks:
    Issue #2084979 by jantoine | impleri: Fixed flexslider_picture() broken...
Ravenight’s picture

Status: Fixed » Needs review

I just upgraded from Picture 1.2 to 1.3 today and received errors "flexslider_picture_optionset" was missing. I dug a bit deeper and found that the #41 patch was not applied on the latest version of flexslider_picture.

It appears this was done:

diff --git a/flexslider_picture/flexslider_picture.module b/flexslider_picture/flexslider_picture.module
index 92043f0..d0b1447 100644
--- a/flexslider_picture/flexslider_picture.module
+++ b/flexslider_picture/flexslider_picture.module
@@ -12,8 +12,8 @@ function flexslider_picture_theme_registry_alter(&$registry) {
   $registry['flexslider_list']['file'] = 'flexslider_picture.theme.inc';
   $registry['flexslider_list']['theme_path'] = drupal_get_path('module', 'flexslider_picture') . '/theme';
   $registry['flexslider_list']['function'] = 'theme_flexslider_picture_list';
-  $default_preprocessor = array_search('template_preprocess_flexslider_list', $registry['flexslider_list']['preprocess functions']);
-  $registry['flexslider_list']['preprocess functions'][(int) $default_preprocessor] = 'template_preprocess_flexslider_picture_list';
+  $default_processor = array_search('template_process_flexslider_list', $registry['flexslider_list']['process functions']);
+  $registry['flexslider_list']['process functions'][(int) $default_processor] = 'template_process_flexslider_picture_list';
   $registry['flexslider_list']['includes'][] = $registry['flexslider_list']['theme_path'] . '/' . $registry['flexslider_list']['file'];
 }

and this was done:

diff --git a/flexslider_picture/theme/flexslider_picture.theme.inc b/flexslider_picture/theme/flexslider_picture.theme.inc
index b93f0aa..ee7627b 100644
--- a/flexslider_picture/theme/flexslider_picture.theme.inc
+++ b/flexslider_picture/theme/flexslider_picture.theme.inc
@@ -7,13 +7,13 @@
 
 
 /**
- * Preprocess the items and prepare the item slides to be rendered.
+ * Process the items and prepare the item slides to be rendered.
  *
  * @param $vars
  */
-function template_preprocess_flexslider_picture_list(&$vars) {
+function template_process_flexslider_picture_list(&$vars) {
   // Call default preprocessor first.
-  template_preprocess_flexslider_list($vars);
+  template_process_flexslider_list($vars);
 
   $optionset = &$vars['settings']['optionset'];

but the part of #41 that involves the flexslider_picture.install was not applied nor some of the other major things in the patch. Just an FYI that you will have to re-apply this patch to the 1.3 version of pictures' flexslider_picture if you find you are getting errors after the update. I did this and now my site is working again.

If #41 should have been multiple patches, that should be resolved in separate issues.

attiks’s picture

Can you create a new patch?

robcarr’s picture

Had been using the patch at #17 and working fine.

Upgraded module to 1.3 and was riddled with errors. Thought I'd upgrade to 7.x-2.1 and greeted with WSOD (Call to a member function getMappings() on a non-object in .../picture/picture.module on line 1062). Although this is a separate issue related to mappings #2274661: Fatal error: Argument 1 passed to picture_get_mapping_breakpoints() must be an instance of PictureMapping I've got no way of rolling/testing out a new patch, so it's worth flagging up here in case anyone else is trying to looking into this Flexslider issue and hits the same brick wall.

robcarr’s picture

Status: Needs review » Needs work

A new patch is required for review

robcarr’s picture

StatusFileSize
new23.17 KB

I've attached a patch rolled against 7.x-2.x... however, I can't test it due to the problem at #2274661: Fatal error: Argument 1 passed to picture_get_mapping_breakpoints() must be an instance of PictureMapping. Let me know if it works!

indigoxela’s picture

Hi,
I knew, #17 is sort of dangerous. It dropped database table flexslider_picture_optionset.

How can we proceed safely? Especially all users who already applied #17 might be forced to completely uninstall and re-install this submodule. Even then eventually direct interaction with the database might be required.

Any ideas? das-peter?

For those who struggle with missing db table errors (PDOException: SQLSTATE[42S02]: Base table or view not found), here is the table structure to recreate it in MySQL (empty!) as sql command:

CREATE TABLE IF NOT EXISTS `flexslider_picture_optionset` (
  `id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'The internal identifier.',
  `flexslider_optionset` varchar(255) NOT NULL COMMENT 'The machine-readable option set name.',
  `imagestyle_type` varchar(255) NOT NULL COMMENT 'One of image_style or picture_mapping.',
  `mapping` varchar(255) DEFAULT NULL COMMENT 'The picture mapping for this optionset.',
  PRIMARY KEY (`id`),
  KEY `imagestyle_type` (`imagestyle_type`)
) ENGINE=InnoDB  DEFAULT CHARSET=utf8 COMMENT='Saves which flexslider optionsets use picture mappings and…' AUTO_INCREMENT=3 ;

Do not apply this if you don't know what the above code means!

jelle_s’s picture

jelle_s’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
jelle_s’s picture

Status: Needs work » Fixed
das-peter’s picture

Awesome, thank you all very much for continuing working on this!! :)

indigoxela’s picture

Hi Jelle_S,
I reverted all changes on my testing instance made by patches created here.
I can confirm that all error messages are gone after updating picture to 7.x-2.1+8-dev.

Unfortunately this doesn't mean, that flexslider_picture is fully functional.
I really don't want to discourage you but...

I checked

  • flexslider optionset
  • breakpoints
  • picture mappings
  • content type display settings

and all looked OK.

Markup looks good, "picture" and "source" tags are set correctly.
(Although I would like to mention that fallback images won't ever work as they don't have a src attribute. But that's a picture issue and is not introduced with this submodule.)

Now the problem:
The image_style actually in use for flexslider displays is the FIRST in image styles list of the picture mappings
(/admin/config/media/picture/list/MAPPING/edit).
This style is also the first "srcset" attribute.
When resizing the screen, no adaption takes place, still the first image of the mapping is used.

All the other picture mappings work just fine, only the flexslider picture responsive images are not responsive.

Am I missing something?

jelle_s’s picture

@indigoxela: Have you cleared your caches? (Drupal & browser cache). Everything works fine on my end.

indigoxela’s picture

Hi Jelle_S,
thank you for your quick response.

Yes, I flushed (and flushed and flushed...) Drupal cache and disabled browser cache.
Still the same problem in Firefox and Chromium.
No javascript errors or php errors at all, only the wrong image size.

Which flexslider library version are you using?
I am using the version suggested here: https://drupal.org/node/1420924#comment-7983425

jelle_s’s picture

indigoxela’s picture

Hi Jelle_S ,
first I have to correct my bug report (comment #57). After the picture update even "normal" picture mappings only use the first "srcset".
So this is likely not a flexslider_picture problem but a picture issue.

Again I reverted everything and started from scratch.
And I switched to the exact same flexslider library you use.
Same result - wrong image size.
And - yes - flushflushflush.

As the html source code looks ok and there are no php errors, it is likely a javascript problem (still no errors though). Picturefill sets the image src attribute, right?
Is there another picturefill version I could try?

Should I try a different picture (dev-)version?
What I've used so far is the official 7.x-2.x-dev version available on the project page.
Maybe I'd better start with 7.x-1.x-dev.

But let us wait a bit, what results other testers get.

attiks’s picture

Keep in mind that the dev versions are build every 12 hours, so you have to wait a bit or use git clone

jelle_s’s picture

Status: Fixed » Active

@indigoxela: can you also print the output of the picture HTML here?

Keep in mind that the least specific breakpoint should appear as the last source element. So in stead of:

<picture>
  <source media="(min-width:0px)" ...>
  <source media="(min-width:400px)" ...>
</picture>

it should be:

<picture>
  <source media="(min-width:400px)" ...>
  <source media="(min-width:0px)" ...>
</picture>

Because picture selects the first matching source element.

You can change the order of the breakpoints at admin/config/media/breakpoints/groups/breakpointgroup
Where breakpointgroup is the machine name of the breakpoint group you selected for your picture mapping.

indigoxela’s picture

@Jelle_S
excellent!

I had to change the breakpoint order to make it work.

the least specific breakpoint should appear as the last source

In other words, the largest image should be the first. When did that change?

It used to be the other way round and still is in the documentation:
https://drupal.org/node/1904690

Now I can confirm that the update from picture 7.x-1.2 to 7.x-2.1+8-dev works.

One caveat: fallback images don't work yet as the img tag has no "src" attribute without javascript.
Keep in mind that flexslider thumbnails won't work without functional fallback images.

attiks’s picture

Fallback without js does indeed not work, but if we add a src on the img tag, it will download 2 images.

What is the flexslider problem with thumbnails?

indigoxela’s picture

@attics:

Fallback without js does indeed not work, but if we add a src on the img tag, it will download 2 images.

Not if you use a "noscript" tag around the fallback image tag. OK, that would require some more html code, but I suppose, a functional fallback solution is desirable.

What is the flexslider problem with thumbnails?

Flexslider library and as a result flexslider module completely changed thumbnail handling.
The problem was discussed here and is also discussed in flexslider issues (e.g. https://drupal.org/node/2123947).

In short: thumbnail image style settings are skipped and instead of that the slider images will be resized via css.
Flexslider module uses preg_match() to grab the first src attribute found for thumbnail html code creation.

attiks’s picture

Can you create a new issue for "Not if you use a "noscript" tag around the fallback image tag"

Flexslider module uses preg_match() to grab the first src attribute found for thumbnail html code creation.

This sounds like a strange approach, since I don't use the flexslider module no idea on how to solve this, best to handle in a new issue as well

PS: Thanks all for all the testing

attiks’s picture

Status: Active » Fixed
jelle_s’s picture

Flexslider module uses preg_match() to grab the first src attribute found for thumbnail html code creation.

This is already fixed, see http://cgit.drupalcode.org/picture/tree/flexslider_picture/theme/flexsli...

das-peter’s picture

Status: Fixed » Active

Something is strange here. It looks like essential parts of the patch weren't committed.
A good example is flexslider_picture_field_formatter_info
What's in the repo:

function flexslider_picture_field_formatter_info() {
  return array(
    'flexslider' => array(
      'label' => t('flexslider'),
      'field types' => array('image', 'media'),
      'settings' => array(
        'optionset' => 'default',
        'image_style' => '',
        'caption' => FALSE,
      ),
    ),
  );
}

What the patch contains:

function flexslider_picture_field_formatter_info() {
  return array(
    'flexslider_picture' => array(
      'label' => t('Flexslider Picture'),
      'field types' => array('image', 'media'),
      'settings' => array(
        'optionset' => 'default',
        'image_style' => '',
        'fallback_image_style' => '',
        'caption' => FALSE,
        'colorbox' => array(
          'enabled' => FALSE,
          'image_style' => '',
          'fallback_image_style' => '',
        ),
      ),
    ),
  );
}

Was this done on purpose? There was no objection related to those parts, right?
As far as I can tell the re-roll of arrrgh in #51 should have been committed as is.

jelle_s’s picture

Assigned: Unassigned » jelle_s

I'll have a look

jelle_s’s picture

Ok, first of all, the patch can not be committed as-is.
As said in #17, it breaks existing configuration without an upgrade path:

  1. It renames the field formatter without updating the existing formatters
  2. It deletes the table holding our current configuration without updating it to the new formatter

The only difference in features I can see, is the colorbox support. So for now, I'm inclined to add the colorbox support without rewriting the entire module. The original issue was to fix the module so it would work with flexslider >= alpha3. A major rewrite can be considered in a separate issue, if there is a decent upgrade path.

das-peter’s picture

Thanks for looking into that - totally missed #17 then, my bad. :|

jelle_s’s picture

Assigned: jelle_s » Unassigned
Status: Active » Fixed

Colorbox is now supported (it was before as well, but with fixing this issue something got messed up).

das-peter’s picture

Status: Fixed » Needs review
StatusFileSize
new12.65 KB

Can't figure out a proper upgrade path right now, so the attached patch just introduces the new approach besides the "legacy" one.

jelle_s’s picture

Status: Needs review » Fixed

@das-peter: Thank you for working on this, but could you move it to a separate issue? The original issue is now fixed. It's more clear if we could keep things separated.

das-peter’s picture

Status: Fixed » Closed (fixed)

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