JQuery Version check for equal or greater than 1.7 bug found in the .module file:
if ( floatval($matches[1] .'.'. $matches[2]) >= 1.7) { // line #37...}
will return false for any version of JQuery greater than 1.9. For example:

The code about will return false for JQuery version 1.10 or 1.11 or 1.12 ,etc since 1.7 > 1.10

Here is my propose fix:
if ($matches[1] >= 1 && $matches[2] >= 7) { ... }
this should fix the problem.

Fred.

CommentFileSizeAuthor
#8 bootstrap_library-jquery_version_check-2244553-8.patch716 bytesJamesAn
#6 bootstrap_library-jquery_version_check-2244553-6.patch818 bytesbdgreen
#4 bootstrap_library-jquery_version_check-2244553-4.patch861 bytesbdgreen
#1 bootstrap_library-jquery_version_check-2244553-1.patch1 KBbarraponto
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

barraponto’s picture

barraponto CreditAttribution: barraponto commented
Title: Bug found in the module » Jquery version check fails for 1.10+
Version: 7.x-1.2 » 7.x-1.x-dev
Status: Active » Needs review
FileSize
bootstrap_library-jquery_version_check-2244553-1.patch1 KB

Here' s a patch.

lhridley’s picture

lhridley CreditAttribution: lhridley commented

Note: Conversion of the version number to a float value is an invalid comparison when checking software versions.

Confirmed patch fixes version check issue with jQuery; however:

A better check would be to use the PHP function version_compare(). This would eliminate the need for the regular expression altogether. Using version_compare(), the block f code checking the jQuery library version would be:

    // Check if the site is running >= jQuery 1.7
    $library = drupal_get_library('system', 'jquery');
    if ( version_compare($library['version'], '1.7', '>=')) {
      $requirements['boostrap_library_jquery'] = array(
        'title' => t('Bootstrap version'),
        'value' => t('jQuery @version', array('@version' => $library['version'])),
        'severity' => REQUIREMENT_OK,
      );
    }
    ...
lhridley’s picture

lhridley CreditAttribution: lhridley commented
Status: Needs review » Needs work
bdgreen’s picture

bdgreen CreditAttribution: bdgreen commented
Status: Needs work » Needs review
FileSize
bootstrap_library-jquery_version_check-2244553-4.patch861 bytes

Patch following on from @lhridley comment #3

Status: Needs review » Needs work

The last submitted patch, 4: bootstrap_library-jquery_version_check-2244553-4.patch, failed testing.

bdgreen’s picture

bdgreen CreditAttribution: bdgreen commented
Status: Needs work » Needs review
FileSize
bootstrap_library-jquery_version_check-2244553-6.patch818 bytes

So patching contrib modules ... (node/707484) - resubmitted

lhridley’s picture

lhridley CreditAttribution: lhridley commented
Status: Needs review » Reviewed & tested by the community
JamesAn’s picture

JamesAn CreditAttribution: JamesAn commented
Status: Reviewed & tested by the community » Needs review
FileSize
bootstrap_library-jquery_version_check-2244553-8.patch716 bytes

Two very minor coding style correction. As per the Drupal coding standards, control statements should not have any whitespace between the opening parenthesis and the first condition (i.e. like this: "if (version...", not like this: "if ( version..."). Also, the comment line should maintain it's indent level; I'm not sure why that change is being proposed/accepted.

Since I'm pointing out these issues, I've rolled the patch with these two cosmetic fixes.

  • hatuhay committed f64d9f8 on 7.x-1.x authored by JamesAn 
    Issue #2244553 by bdgreen, JamesAn, barraponto: Jquery version check...
Pol’s picture

Pol
the/that
Location Brussels
CreditAttribution: Pol as a volunteer commented
Status: Needs review » Active

Why not creating a new release with this patch included ?

bneil’s picture

bneil CreditAttribution: bneil at The University of Iowa commented
Status: Active » Closed (fixed)

Setting to fixed since this was committed.

nandt’s picture

nandt CreditAttribution: nandt commented

apply patch #8 solve the problem.
thanks

aaronsthomas74’s picture

aaronsthomas74 CreditAttribution: aaronsthomas74 commented

Patch # 8 worked for me too.