Just a note that I am aware of the Libraries API.
When I introduced the dependency on jQuery Timers I thought about integrating with Libraries API. I did not understand, though, what Libraries API currently does, so for simplicity I just released without it. I should have put the jQuery Timers library in sites/all/libraries instead of directly in the clock module directory, but that's too late now.
I am thinking about writing some code to nudge people in the UI to move the library, but I not convinced that it's actually worth it, without true Libraries API integration.
I appreciate any information, help, comments, etc. in this direction!

Marking postponed on a stable release of Libraries API for now.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

Issue tags: +Libraries

Let's join the crowd

tstoeckler’s picture

Assigned: Unassigned » tstoeckler
Status: Postponed » Active

I think we should do three releases of Clock:
1. nudge people that /libraries is a better directory on /admin/block/configure/clock/clock with a 'warning' message telling them to move because the next release will not support it. Also tell them to not rename the file anymore.
2. tell people that /clock is WRONG! on /admin/block/configure/clock/clock with an 'error' message telling them that this is unsupported also make the JS-part not work (maybe hide the checkbox). Also show a similar error if the file is in /libraries but they have renamed it.
3. Remove all handling of the duplicate directories. Just include the file at /libraries and be done with it. People that haven't moved their file yet, get some weird PHP error probably. This will allow #720376: Add a Drush Make file.

I'm also thinking that this should go hand in hand with dependecy on Libraries API. Maybe in the following order (the numbers are the same as above).
1. In the warning message (and the release notes) include a note that Clock will depend on Libraries API in the future, so it's best to go get it now.
2. Do not do a dependencies[] = libraries in the .info file, but show a warning if Libraries API is not installed regardless where the library is.
3. dependencies[] = libraries

Will upload a patch for 1 soon.

tstoeckler’s picture

Worked on 1 and got it working, but there is one problem.
jQuery Timers is distributed as a .js.txt files for whatever reasons.
drupal_add_js() didn't work for me with that file, only when renaming it to .js. I will investigate whether
1. jQuery Timers people are weird for letting us download a .js.txt file
2. Drupal is weird for not handling .txt files as JS.
3. We will have to rename the file until the end of time.

tstoeckler’s picture

So, I clicked around a little.
It seems core does rename the files from .js.txt to .js. I could find no documentation whatsoever about why jQuery will only allow .js.txt downloads, but it seems consistent with all plugins I looked at. I'm guessing security of their website, in case the JS is corrupt. I also could not find any source code of jQuery Timers or any form of Version control it is in.
I do not know whether it is actually a bug, for Drupal not to handle .js.txt files. Actually know that I think of it. The file was added to the scripts, I just got an error, maybe that was even browser-related. I don't know.
Anyways, I don't think it can be considered best practice to have a JS file with .txt ending.
Therefore we need to rename it.
In the past we also removed the version string from the filename. I looked and it seems, this is an inconsistency in jQuery Timers. Other jQuery libraries do not include the version string in their filename.

Therefore:
I hereby proclaim that is the Right Thing (c) to rename jquery.timers-1.2.js.txt to jquery.timers.js and will be of this belief until proven otherwise.

Working on a patch now.

tstoeckler’s picture

FileSize
3.56 KB

Will commit this in a minute.

tstoeckler’s picture

http://drupal.org/cvs?commit=344722

"Back" to active.

When Clock-6.x-1.2 is out we will do part 2.

In the meantime I will commit a patch to Drupal 7 which simply introduces a dependency on Libraries API.
Will update the project page then.

tstoeckler’s picture

Nooooo!!!
Writing that code for D7 I just realized that for Libraries API jQuery Timers will need to live in a 'jquerytimers' or 'timers' folder. I actually think 'timers' is the better name, because it is not 'namespaced', but the title of the project page of jQuery Timers is "jQuery Timers", so 'jquerytimers' is the correct folder name.
Will upload a patch to correct that.

...And then on to D7.

tstoeckler’s picture

This is how it should have been.

tstoeckler’s picture

http://drupal.org/cvs?commit=344726
Committed untested. Remember to run tests before release. (Also: remember to update the translation template before release)

Now on to D7.

tstoeckler’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Active » Needs review
FileSize
1.19 KB

Let's see if it works...

tstoeckler’s picture

Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Needs review » Active

Yup. The patch file contains the wrong info file, but I didn't upload a new patch for that.
http://drupal.org/node/706594

Back to active and D6

tstoeckler’s picture

Now that 1.2 is out, we can approach level 2.

tstoeckler’s picture

Status: Active » Needs review
FileSize
4.52 KB

Untested.

tstoeckler’s picture

FileSize
4.05 KB

Well, I guess removing the form is just not that nice...

tstoeckler’s picture

Status: Needs review » Needs work
+++ clock.module	26 May 2010 10:46:38 -0000
@@ -67,18 +67,17 @@ function clock_block($op = 'list', $delt
+          drupal_set_message('Please place the <a href="http://plugins.jquery.com/project/timers">jQuery Timers library</a> in a dedicated <em>jquerytimers</em> directory inside the <em>libraries</em> directory in your <em>sites</em> directory. The file should then be located at e.g. <em>sites/all/libraries/jquerytimers/jquery.timers.js</em>. Clock module will not function otherwise', 'error');

Missing trailing period.

+++ clock.module	26 May 2010 10:46:38 -0000
@@ -308,18 +307,8 @@ function theme_clock($timezone, $date_fo
+    drupal_add_js(libraries_get_path('jquerytimers') . '/jquery.timers.js');

This will lead to WSOD for people upgrading, who don't have Libraries yet. Should be wrapped in a function_exists().

Powered by Dreditor.

tstoeckler’s picture

Status: Needs work » Postponed

http://drupal.org/cvs?commit=371560

I must have multiple personalities...

Marking postponed, until the next release, where we will add: dependencies[] = libraries and get rid of the cruft.

tstoeckler’s picture

In #853000: Use only 3 variables instead of 4. I accidentally committed "true" Libraries API integration, so that Clock now depends on Libraries >7.x-alpha1. Will need to document that in the info file.

tstoeckler’s picture

Status: Postponed » Active
tstoeckler’s picture

Status: Active » Closed (won't fix)