I was surprised to see this module will not enable at all until the webroot ./js directory includes the colorpicker.js file. That's not really the Drupal way to do things is it? (Legitimate question.)

First of all, I was confused by this message:

"You are missing the required files for the Jquery Colorpicker. Please download the colorpicker.zip from http://www.eyecon.ro/colorpicker/ and unzip the CSS, JS and IMAGES folders to . After doing this, confirm that the following path exists: /js/colorpicker.js."

Phrasing it "unzip the CSS, JS, and IMAGES folders to ." is somewhat confusing -- it would ideally read "unzip the CSS, JS, and IMAGES folders to your webroot." Or something.

Second of all, the module should probably ENABLE and then throw a warning message instead of not enabling at all, right?

Third of all, the module should not look in ./js -- it should either let you choose and ideally default to ./sites/all/libraries/colorpicker/js or something like that. I thought I might just be able to put my files there and then symlink ./js, but I realized that if any other module operated similarly, no matter what I'd have to amalgamate its JS files with Colorpicker's.

Does that make sense? Thank you!

Comments

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Category: task » bug
Priority: Normal » Critical

It's very strange indeed, you have to put this in root /js/ ?

Warning: file_get_contents(/js/colorpicker.js): failed to open stream: No such file or directory in _locale_parse_js_file() (regel 1488 van /var/www/vhosts/123.com/httpdocs/includes/locale.inc).
Warning: file_get_contents(/js/colorpicker.js): failed to open stream: No such file or directory in drupal_build_js_cache() (regel 4846 van /var/www/vhosts/123.com/httpdocs/includes/common.inc).
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Active » Fixed

Problem is fixed by putting the files under /sites/all/libraries/colorpicker/js/colorpicker.js etc.

Offlein’s picture

Offlein CreditAttribution: Offlein commented
Priority: Critical » Normal
Status: Fixed » Active

I don't know if I'd call this critical (or fixed) -- perhaps you can explain what happened?
You're saying that you don't have to put it in root /js -- that if you put it in /sites/all/libraries/colorpicker/js it still works out of the box?

In that case, I think the Documentation needs work.

plopesc’s picture

plopesc
he/him
Primary language Spanish
CreditAttribution: plopesc commented
Status: Active » Fixed

Hello

Both in the project page (project) and in the docummentation page (doc), is described the place where the jquery_colorpicker library must be placed.

Important

Since version 7.x-1.0-rc1, the module depends on libraries API, then you have to move the JQuery library from the module folder to libraries folder. After upgrading to 7.x-1.0-rc1, theme cache must be rebuilt, given that the module implements new theme functions.

Please read carefully the doc and other issues before opening new ones.

Regards.

Offlein’s picture

Offlein CreditAttribution: Offlein commented
Version: 7.x-1.0-rc1 » 7.x-1.x-dev
Status: Fixed » Active

Understood. Please allow me to respectfully suggest that there is still an issue, however.

When I decide I want a module -- and I suspect many users may operate this way -- I quickly "drush dl" it and any dependencies, and enable it. And some point either before or after I'll take a quick look at the project page to see if I need to do any custom configuration, or else the module will throw a drupal_set_message() warning that a required library has not yet been downloaded when viewing an admin page. (It is non-standard, I believe, to stop a user from actually enabling the module. But I do see how this makes sense to do.)

The real issue is that, when the module fails to enable, it throws an error with wrong information. In jquery_colorpicker.install you do $path = libraries_get_path('colorpicker'); to find the directory where the 'colorpicker' library lives. If the library is not there (and hence $path is empty) you go on to tell the user (in Line 98) to download colorpicker and unzip it into $path (which, again is blank) and hence your message tells the user to unzip the files into " " -- just like the error I received above.

I looked briefly and there didn't seem to be a good, quick way to determine programmatically where all the possible Libraries paths are, so I would just suggest that you hard code the error message to use path "/sites/all/libraries/" -- which is a common (and hard-coded in libraries.module) search path to be checked.

Does that make sense? I would have sought out the documentation you mention had the module not explicitly told me where to put the files.

Offlein’s picture

Offlein CreditAttribution: Offlein commented
Title: Why does ColorPicker require library to be in root ./js directory? » ColorPicker error message gives wrong path when enabling the module without the colorpicker library
Component: Code » Documentation
Priority: Normal » Minor

Changing issue info.

plopesc’s picture

plopesc
he/him
Primary language Spanish
CreditAttribution: plopesc commented

Hello

You're right Libraries 1.x branch librariees_get_path() returns by default "sites/all/libraries/library_name" if the library does not exists. However in 2.x branch, it returns FALSE.

Changing the code to provide 2.x branch support.

Thanks

plopesc’s picture

plopesc
he/him
Primary language Spanish
CreditAttribution: plopesc commented
Status: Active » Fixed

Commited in http://drupalcode.org/project/jquery_colorpicker.git/commit/be4716b
Thanks

Offlein’s picture

Offlein CreditAttribution: Offlein commented

Thank YOU!

2pha’s picture

2pha
Primary language English
CreditAttribution: 2pha commented

I think some install instructions and dependencies should be outlined in a README.txt file as outlined by the drupal module best practices.

alexweber’s picture

alexweber CreditAttribution: alexweber commented

Event though this has been fixed its pretty annoying. The best way to handle dependencies is through hook_requirements() and not blocking a user from enabling the module until everything's been fixed. It's pretty frustrating...

plopesc’s picture

plopesc
he/him
Primary language Spanish
CreditAttribution: plopesc commented
Category: bug » feature
Status: Fixed » Needs work

Hello

Thanks for your interest in the module. In my opinion, the bug is fixed and this issue should be marked as a feature request.

I'll try to work in this direction, but meanwhile, patches are welcomed.

Regards

jvdurme’s picture

jvdurme CreditAttribution: jvdurme commented

Not sure what goes wrong, but I can't enable this module even when I have the correct library installed in the correct location.
I have the file path sites/all/libraries/colorpicker/js/colorpicker.js

But when I enable the module I get:

jQuery Colorpicker not installed
You are missing the required files for the Jquery Colorpicker. Please download the colorpicker.zip from http://www.eyecon.ro/colorpicker/ and unzip the CSS, JS and IMAGES folders to sites/all/libraries/colorpicker. After doing this, confirm that the following path exists: sites/all/libraries/colorpicker/js/colorpicker.js.

Have I missed something?

Diego Balboa’s picture

Diego Balboa CreditAttribution: Diego Balboa commented

jvdurme: I had same problem.............................
I FORGOT TO CHANGE FILE RIGHTS (yes ¬¬)

Try granting read/write permissions to all users to all files and folders contained at sites/all/libraries/colorpicker ... and give it a try.

If those are restricted (eg: u are using root user under linux in order to move files to /var/www folder , but you are using web navigator under non root without permissions) user.

the script cant access the file when it checks if such a file exist, at line 93:

if (!is_file($path . '/js/colorpicker.js')) {

so it reports that error..

I spent 2 hours reviewing the jquery_colorpicker.install file searchinf for an error or a hint only to realize I FORGOT to check permisions. Always check the simplyest the sooner ;)

Jaypan’s picture

Jaypan CreditAttribution: Jaypan at Jaypan commented
Issue summary: View changes
Status: Needs work » Fixed

Nice debugging Diego!

Alex Weber - at some point hook_requirements() was added to the code. So I'm marking this issue as fixed.

Status: Fixed » Closed (fixed)

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