Use the Libraries module

I have started working on this feature in the new feature integration branch (7.x-2.x currently, but if there is a demand for that then I can backport it to D7) based on your patch.

I would like to give a proper integration to the libraries module. The problem is that the libraries module wants to include the modernizr.js file directly, but it should not be included directly. The reason is that in order to get the Modernizr triggered, a 'no-js' class should be on the html tag, but in Drupal, there is no way to put classes on the element. So I have created a little helper, which adds the required classes to the html tag and then loads the library.

I have to think about this for a few days, but I try to come up with a smart solution. Meanwhile, I commit the code what I did to CVS.

Would love to see this included in the 6.x branch. Libraries is a godsend for those of use that maintain several sites.

Can someone verify that the patch works correctly with the D6 version? If so, then I will merge to the -dev version.

+1 for this.

The reason is that in order to get the Modernizr triggered, a 'no-js' class should be on the html tag, but in Drupal, there is no way to put classes on the element.

Could just add an instruction to add 'no-js' to html.tpl.php (D7) and page.tpl.php (D6)?

I wouldn't do it, because my html.tpl.php and page.tpl.php can be overwritten by a different module or theme or modernizr can break a theme or a module, which expects a special html.tpl.php or page.tpl.php.

The patch worked great for me, however I noticed they aren't including a .min.js file in the version 1.7 package, but rather just calling it modernizr.js .

So instead of renaming the 1.7 version back to modernizr-1.6.min.js -I've adjusted the patch to call for modernizr.js

here's the commands that worked in the end for the patch above

cd sites/all/modules/modernizr
wget http://drupal.org/files/issues/modernizr-libraries.patch
patch < modernizr-libraries.patch
rm modernizr-libraries.patch
cd ../../libraries
wget https://github.com/Modernizr/Modernizr/zipball/master --no-check-certificate
unzip master
rm master
mv Modernizr-Modernizr-ff9562a/ modernizr-1.6.min.js

Will test the patch that I've attached and post results.

The new patch worked for me again, should mention this is with the latest dev release of modernizr module.

Here's the commands updated for above

cd sites/all/modules/modernizr
wget http://drupal.org/files/issues/modernizr-libraries_0.patch
patch < modernizr-libraries_0.patch
rm modernizr-libraries_0.patch
cd ../../libraries
wget https://github.com/Modernizr/Modernizr/zipball/master --no-check-certificate
unzip master
rm master
mv Modernizr-Modernizr-ff9562a/ modernizr

Unless of course I've missed the minified version somewhere in the 1.7 modernizr library. But it really wasn't there.

+1

...subscribing for a 7.x patch/solution - when available.

Instructions on #10 worked for me in D6. Could we get this added to the dev release?

+1

Patch merged in commit 081afa72560ed9a7c180af3b068fb915530f182a.

Please test it, and if it works, I'll release the 6.x-1.1 version.

For D7, the 7.x-2.x beta is really close, and that version fixes this problem also.

Thanks for the patience.

I was getting bad calls to the directory "sites/all/libraries/modernizr" instead of the fully qualified path to the file. The attached patch got the requests loading properly. This still seems a bit inflexible because it only looks for modernizr.min.js or modernizr.js

Could we use the regex approach that's present in the 7.x branch to load any permutation that we can usually expect to find?

modernizr.js
modernizr.min.js
modernizr.custom.js

I just tested with 7.x-2.1 and discovered that placing the file in /profiles/nameofprofile/libraries/modernizr/modernizr.min.js doesn't work.

Instructions say to place it in sites/.../libraries/modernizr/modernizr.min.js, but the libraries module should be able to load libraries from within an install profile. The libraries module loads my other libraries in my profile just fine.

Any thoughts on what could be wrong?

Nevermind, I ran it again and it worked fine.

Ok, cool. I had started looking into this but could not reproduce, glad to hear it's working for you!

In modernizr.module, I think we need to change this function

function modernizr_library_info() {
  $libraries = array();

  $libraries['modernizr'] = array(
    'title' => t('Modernizr'),
    'vendor url' => 'http://modernizr.com',
    'download url' => 'http://modernizr.com',
    'version arguments' => array(
      'file' => 'modernizr.min.js',
      'pattern' => MODERNIZR_VERSION_REGEX,
    ),
    'files' => array(
      'js' => array(
        // This array is intentionally left blank.
      ),
    ),
    'integraton files' => array(
      'modernizr' => array(
        'js' => array(
          drupal_get_path('module', 'modernizr') . '/modernizr_loader.js',
        ),
      ),
    ),
  );

  return $libraries;
}

I think this should be

function modernizr_libraries_info() {
  $libraries = array();

  $libraries['modernizr'] = array(
    'name' => t('Modernizr'),
    'vendor url' => 'http://modernizr.com',
    'download url' => 'http://modernizr.com',
    'version arguments' => array(
      'file' => 'modernizr.min.js',
      'pattern' => MODERNIZR_VERSION_REGEX,
    ),
    'files' => array(
      'js' => array(
        // This array is intentionally left blank.
      ),
    ),
    'integraton files' => array(
      'modernizr' => array(
        'js' => array(
          drupal_get_path('module', 'modernizr') . '/modernizr_loader.js',
        ),
      ),
    ),
  );

  return $libraries;
}

In the function above I changed two spots: modernizr_library_info() should be modernizr_libraries_info() and the "title" attribute should be "name".

Patch is attached. The patch should be applied to the 7.x-2.x branch. Hope that helps,
-Glenn

Good catch and thanks, Glenn! Committed to 7.x-3.x

I haven't done it yet, but I will do more clean up on the implementation of this function. Modernizr 2.x doesn't require the .no-js class, so it can load directly with a <script> tag, making the modernizr_loader.js business less of an issue.