Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jQuery version check fails for versions > 1.10 #9

Open
rohnjeynolds opened this issue Dec 23, 2015 · 4 comments
Open

jQuery version check fails for versions > 1.10 #9

rohnjeynolds opened this issue Dec 23, 2015 · 4 comments

Comments

@rohnjeynolds
Copy link

In the OoyalaUploader.jsonParsePolyfill() method, the check for jQuery version intends to check for versions lower than 1.5, but the check also returns true for jQuery versions 1.10 or greater:

    OoyalaUploader.jsonParsePolyfill = function(data) {
      if (jQuery.fn.jquery < "1.5") {
        return JSON.parse(data);
      }
      return data;
    };

As a result, JSON.parse() is called on an already parsed object, a parser error is thrown and upload fails. I worked around by changing the conditional to:

      if (jQuery.fn.jquery < "1.5" && jQuery.fn.jquery < "1.10") {
@quicksketch
Copy link

Hi @rohnjeynolds, I'm checking out all the issues with Ooyala module for Drupal and encountered this issue, which is related to this: https://www.drupal.org/node/2469013

It's worth noting that the jsonParsePolyfill() method is not actually in this version of the library. It's only the Lullabot fork of the project:

Compare:
https://github.com/ooyala/backlot-ingestion-library/blob/master/ooyala_uploader.js#L171
https://github.com/Lullabot/backlot-ingestion-library/blob/master/ooyala_uploader.js#L171

So the upstream library does not actually have this problem. It looks like if the Drupal module could deal with either format, we'd save ourselves the need of the fork in the first place.

But in any case, this issue isn't valid against this project, because it doesn't contain the line in question in the first place.

@rohnjeynolds
Copy link
Author

Thanks, @quicksketch. I don't recall how this happened, but I remember being confused about the different versions of the library. I initially used this version but later I think I found that the README file in the Ooyala Uploader module lists the URL of the Lullabot fork as a requirement. (http://cgit.drupalcode.org/ooyala/tree/ooyala_upload/README.txt#n13)

Either I created the issue here because I didn't recognize that a fork had occurred, or I couldn't figure out how to create an issue on the fork... In any case, it was a blocker for me to get uploading working at the time, and I felt duty-bound to report it somehow.

Very glad to see some activity on the Ooyala module. The Drupal issue you linked implies that the library might be made to support old and new jQuery versions, and rolled with the module itself? Do you have an idea when that might be rolled out into a new, stable version of the Ooyala module?

@quicksketch
Copy link

Do you have an idea when that might be rolled out into a new, stable version of the Ooyala module?

I made a new release just a few days ago that now bundles this library, with the modifications made in the Lullabot fork and should be compatible with all jQuery versions. ☺

@rohnjeynolds
Copy link
Author

Fantastic news! Thanks, @quicksketch !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants