No input validation on pager position input (Fixed)

Can not reproduce this bug.

What browser are you using?
Do you see JavaScript errors at the page, so the JavaScript for the slideshow configuration page is not used.?

I don't think this is a browser/javascript problem (using Firefox 3.6.3 on Windows - no sign of Javascript errors).

I have had this problem as well. I set the pager position to bottom, saved and then displayed the block. Everything was ok (pager was shown at the bottom as configured). When I return to edit the View, the Slideshow settings the configuration form has defaulted to pager position = top.
The form does not seem to be populating the setting from the database when it is loading. The problem only seems to be with this one setting, but I left mosst of the other settings at their default values so I cannot be sure.

I could reproduce the bug now. Will fix the javascript to populate the values.

The problem is that after the dropdown is populated with the old value the js code deletes all items in the dropdown list and then rewrites the options that are valid for the current style (views_slideshow_ddblock\js\views_slideshow_ddblock.admin.js).

I have created a patch that stores the old value before the list is deleted and then restores it after the list is populated with valid items.

If the old value is not in the valid list then it will default to the first item in the list.

Thanks for the patch. I have a little bit different solution.

Committed to CVS, but somehow can not make a new dev release note.
Will try again tomorrow, its late here.

My other solution would have been to have option buttons for all 4 positions and disable the ones that were not available for the current theme. This way you do not delete options so you avoid the problem.

#7 the new dev 2.x version is available now, you can see what I changed, using the pagerPosition variable

I have tried the latest files and you seem to have all items listed for the pager position regardless of whether the options are invalid for the current theme. When invalid selections are made, there is no validation errors to tell you that there is anything wrong. I did my testing with the vsd-upright-60p theme and invalid input for pager position resulted in no image being displayed.

#9

Can not reproduce this.

When I use a portrait theme I only see the options: bottom and top.
When I use a landscape theme I only see the options: left and right.

I have reinstalled the new code.
I ran update.php (not reqlly required)
I flushed the cache
I verified that the new version is installed.

I still have the same issue (all options displayed for pager position). I only have portrait themes installed and it seems to be the case with all of them.

I am running Windows 7 and have checked IE8, Firefox, Chrome, Safari and Opera and they all have the same problem.

The code which determines the pager positions is:

    // portrait themes
    if (val.match("-10p") == "-10p" || val.match("-20p") == "-20p" ||
        val.match("-30p") == "-30p" || val.match("-40p") == "-40p" ||
        val.match("-50p") == "-50p" || val.match("-60p") == "-60p") {
      var pagerPositionOptions = new Array();  
      pagerPositionOptions[0] = new initPagerPositionOptions('top', 'Top');
      pagerPositionOptions[1] = new initPagerPositionOptions('bottom', 'Bottom');
      
      setPagerPositionOptions(pagerPositionOptions, ddblockSettings.pagerPosition);
    }
    // landscape themes
    else {
      var pagerPositionOptions = new Array();  
      pagerPositionOptions[0] = new initPagerPositionOptions('left', 'Left');
      pagerPositionOptions[1] = new initPagerPositionOptions('right', 'Right');
 
      setPagerPositionOptions(pagerPositionOptions, ddblockSettings.pagerPosition);
    }           

I can not think of a way that all four pager-positions can be shown at ones.

You mean you see top, bottom, right and left for the pager-position?

I have done some checking on my install:

JS function setPagerPositionOptions does not run at all. This is because in Drupal.behaviors.ddblockShowHideCustomTemplateOptions ddblockSettings is undefined. I have checked the view edit page in Firebug and Drupal.settings.ddblockCustomTemplate does not exist in the DOM so ddblockSettings is never set.

I have checked the source for a ddblock page and it defines ddblockCustomTemplate as part of a jQuery.extend statement after the script definitions. The view edit page does not contain this definition (the jQuery statement is there but not the ddblockCustomTemplate definition) so it is not present for reference by the Javascript.

As far as I can tell ddblockCustomTemplate is added to the jQuery statement in views_slideshow_ddblock.module (dated 2010/04/25 08:10:34 in my install) and views_slideshow_ddblock.views_slideshow.inc (dated 2010/04/24 04:01:38 in my install)

Hope this helps.

All works as expected at my testserver.

What version of the views_slideshow_ddblock.admin.js file do you see when you click view page source in FF and then open the views_slideshow_ddblock.admin.js in the head part

it should be:

// $Id: views_slideshow_ddblock.admin.js,v 1.2.2.2 2010/07/16 11:59:53 ppblaauw Exp $

That is what I have.

The jQuery.extend should be written into the page by the php code anyway.

I will rebuild my test server later today to see if that fixes the problem.

Someone else has also got this problem (http://drupal.org/node/850168) so it is not just limited to my build.

You have the same error as in this post?

I do not get the error message but that could be due to the type of browser or browser options. The cause is the same; ddblockCustomTemplate is undefined when I print it to the screen and this is why the js to update the pager position dropdown list is not being called.

The ddblockCustomTemplate is set in views_slideshow_ddblock.views_slideshow.inc

function views_slideshow_ddblock_views_slideshow_options_form(&$form, &$form_state, &$view, $form_state_values = array()) {

  // add views_slideshow_ddblock.admin. js file
  drupal_add_js(drupal_get_path('module', 'views_slideshow_ddblock') . '/js/views_slideshow_ddblock.admin.js');

  // set pager variable for javascript
  $settings['ddblockCustomTemplate'] = array(
    'pager' => $view->options['views_slideshow_ddblock']['settings']['pager_settings']['pager'],
    'pagerPosition' => $view->options['views_slideshow_ddblock']['settings']['pager_settings']['pager_position'],
  );

  drupal_add_js($settings, 'setting');

Don't know how it could be undefined

I have been switching between the previous and current dev versions and I have an idea what is going on (just a theory):

Drupal.settings.ddblockCustomTemplate which is copied to ddblockSetting in the js is not visible in the DOM in Firebug with the previous or current versions of the module.

You can only see Drupal.settings.ddblockCustomTemplate on pages where a Dynamic Display Block is visible. I have my test set up as a page so Drupal.settings.ddblockCustomTemplate is not visible in Views. If I set up a test as a block and do not stop it being displayed on my Views page then Drupal.settings.ddblockCustomTemplate is visible in the DOM and everything looks ok - I think this is the case on your test server, please confirm.

I have tried to track why this is not working. I can print the values (via drupal_set_message) that are put into $settings prior to the drupal_add_js statement and that is ok (when creating ddblockCustomTemplate in the .module you actually have 3 values and only 2 here) so they should be added to the DOM via the jQuery.extend statement. The only things I can think of are:

• something is overwriting / resetting this
• that you are not able to create new DOM elements once the page has loaded
• since the page has already loaded you have to do something to trigger this new code and add the elements to the DOM

The fix for the pager position setting in this version will not work even when this bug is fixed. Whenever you edit the settings for the Dynamic Display the pager position dropdown will show whatever the current saved settings are for the block. If a Dynamic display is currently saved with pager position = Top, edit the settings and set the pager position = bottom - Update but do not save the settings. When you go back and check the settings, they have reverted back to top because the views page is using the saved settings to update the dropdown.

There are even more problems if you have one Dynamic Display block visible on the Views page and are editting the settings for another.

Yes, I can confirm that I have a slideshow on the views page and when I disable this I can now reproduce the issue.

Big step forward in finding the cause and a solution.

Thanks

set status to active

ddblockSettings is only used to set the pager position for the dropdown list when the user wants to change the settings. From what I have said in #20, this will not work even if we get ddblockSettings populated with data:

The fix for the pager position setting in this version will not work even when this bug is fixed. Whenever you edit the settings for the Dynamic Display the pager position dropdown will show whatever the current saved settings are for the block. If a Dynamic display is currently saved with pager position = Top, edit the settings and set the pager position = bottom - Update but do not save the settings. When you go back and check the settings, they have reverted back to top because the views page is using the saved settings to update the dropdown.

A solution to this would be to revert the js to the previous dev release and then apply the patch from #5. The patch reads the value that is populated into the pager position before the dropdown list is emptied and then sets the value once the list is populated with the correct values. This completely bypasses the problem with ddblockSettings.

The two other lines in Drupal.behaviors.ddblockShowHideCustomTemplateOptions in the old js file that reference ddblockSettings should be deleted manually to prevent js errors in some browsers.

Agree, this will work. Thanks.
Will create a patch tomorrow.

Patch to fix that pager position sticks and only uses valid options.
Apply to latest 2.x-dev release (2010-Jul-16)

Had to patch manually because gnuwin32/patch has problems working with windows 7 (trying to get an alternative now).

Works fine.

committed.