After updating our version of the flickr module, we started getting javascript errors about 'url is undefined' in the flickr_colorbox.js file. This was also causing all other javascript (superfish, panelizer, etc.) to not work. Patch will be forthcoming.

CommentFileSizeAuthor
#10 check_for_class_colorbox_plus_respect_colorbox_path_settings-2301681-10.patch874 byteslolandese
#1 url_is_undefined-2301681.patch1.01 KBlunazoid

Comments

lunazoid’s picture

lunazoid commented
StatusFileSize
new url_is_undefined-2301681.patch1.01 KB

Here's a patch that we were able to use to restore our javascript. It just adds a conditional to make sure the url variable is valid.

  • lolandese committed b45767d on 7.x-1.x authored by lunazoid 
    Issue #2301681 by lunazoid: Added conditional to make sure Javascript...
lolandese’s picture

lolandese commented
Status: Active » Fixed

Tested and committed as the provided conditional is harmless in any case and avoids breaking stuff for an existing use case that might affect also others.

Just to understand the cause, in Chrome, can you:

  • open a Flickr image with Colorbox (you use Colorbox right?)
  • right click the image and select 'Inspect element'
  • right click the highlighted image element in the inspector window and 'Copy as HTML'
  • paste it to this issue.

It should look like:
<img class="cboxPhoto" src="https://farm4.static.flickr.com/3771/9244604259_59708a8cf4.jpg" style="cursor: pointer; width: 371px; height: 500px; float: none;">

I suspect your class is different than the one in my example. Do you have non-Flickr images on the same page that open with Colorbox? If so, can you send the HTML of those opened in Colorbox as well? Or if you know the cause, can you enlighten me directly?

Thanks for reporting, the patch and your feedback.

lolandese’s picture

lolandese commented
Related issues: +#2262349: Add a link to Flickr to the overlay browser
lunazoid’s picture

lunazoid commented

Not currently using colorbox for the flickr images, which is probably where the problem comes from.

lolandese’s picture

lolandese commented

Not currently using colorbox for the flickr images, which is probably where the problem comes from.

It does not load flickr_colorbox.js in that case. In flickr.module line 35:

/sites/all/modules/flickr/flickr.module:
   35    if (module_exists('colorbox') && !path_is_admin($_GET['q'])) {
   36      if (variable_get('colorbox_style', 'default') == 'default') {
   37:       drupal_add_js(drupal_get_path('module', 'flickr') . '/flickr_colorbox.js');
   38      }
   39    }

Do you have some alternative JS loading mechanism in place on your site? By-passing drupal_add_js?

EDIT:
flickr_colorbox.js would get loaded when Colorbox is enabled with the default style, but is not used for embedded Flickr photos. It does not invoke an error though. I am still not able to provoke the reported error message which would give a better insight in the underlying cause.

lunazoid’s picture

lunazoid commented

I apologize for not responding sooner. I think I know what the problem might be in our case. We do have colorbox enabled for other purposes, but are not using it for displaying the flickr images. It looks like flickr_colorbox.js will still get loaded in this case:

  if (module_exists('colorbox') && !path_is_admin($_GET['q'])) {
    if (variable_get('colorbox_style', 'default') == 'default') {
      drupal_add_js(drupal_get_path('module', 'flickr') . '/flickr_colorbox.js');
    }
  }

Since colorbox is enabled, but the colorbox_style is not set in our variables table, it will load flickr_colorbox.js. However, in flickr_colorbox.js, var url = $("img[class='cboxPhoto']").attr('src'); will be null, since the image classes don't have the cboxPhoto class. Therefore, just doing the test to make sure url is a valid object before referencing its members fixes it for us.

In the end, we do need to make the switch to using colorbox for the images, which would render all of this a moot point. We just upgraded from a much older version of the library (1.0, I believe,) and I think that version used lightbox instead.

Status: Fixed » Closed (fixed)

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

webservant316’s picture

webservant316 commented

The path above solved my problem. The Flickr Javascript was breaking the function of Flexslider.

Also, seems to me that Flickr could be more careful to only put their javascript on pages that really need it. Why invite unneeded conflicts? This code provides the conditions to adding the Flickr Colorbox to the page...

if (module_exists('colorbox') && !path_is_admin($_GET['q'])) {
if (variable_get('colorbox_style', 'default') == 'default') {
drupal_add_js(drupal_get_path('module', 'flickr') . '/flickr_colorbox.js');
}
}

Why not add further useful conditions? Colorbox has it own restriction system based on paths. Why not respect those restriction here? Also if Flickr fields are enabled and not filters, then why not only add the javascript on pages that use the Flickr field? Or if filters are used then only include the javascript on pages using a filter with the Flickr feature enabled.

Anyway, thanks for the patch above.

lolandese’s picture

lolandese commented
Category: Bug report » Feature request
Status: Closed (fixed) » Needs review
StatusFileSize
new check_for_class_colorbox_plus_respect_colorbox_path_settings-2301681-10.patch874 bytes

Why not add further useful conditions? Colorbox has it own restriction system based on paths. Why not respect those restriction here?

Makes sense. Thanks for these suggestions.

Attached patch implements these.

Still thinking for the best way for the second part of your post.

  • lolandese committed 79ef95c on 7.x-1.x 
    Issue #2301681 by lunazoid, webservant316: Check for the existence of...
lolandese’s picture

lolandese commented
Status: Needs review » Fixed

Simplified and committed.

Status: Fixed » Closed (fixed)

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