* There are many contributed modules in D6 that rely on libraries module to manage external plugins

* Almost all of these modules make a call to libraries_get_path() with only one argument (the lib resource) and do not include the second argument requesting the path to be returned with the base path cosiderations. Eg: http://drupalcode.org/project/filedepot.git/blob/617ebc4a5c95fc751b3a379...

* It makes sense for the libraries module to have the default option of returning the lib path with the base path (and if the basepath needs to be exluded then the second argument can be supplied as FALSE)

I hope that you would agree that this choice of default will mean modules work in all kinds of setups. Please accept the patch if you do :-)

CommentFileSizeAuthor
#1 rerwitebase_default-1561332-1.patch343 bytesSumeet.Pareek
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sumeet.Pareek’s picture

Sumeet.Pareek CreditAttribution: Sumeet.Pareek commented
Status: Active » Needs review
FileSize
rerwitebase_default-1561332-1.patch343 bytes

Here is simple patch.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

That's a bold assumption.

For backend (PHP) libraries, the base path must not be prepended.

For frontend (JS/CSS/Flash/etc) libraries, a default of TRUE might look more sane. But it's only valid if you want to print the path directly in a theme template. Whenever the library path gets passed through url(), the base path must not be prepended.

I do not believe that the topmost use-case is to output library paths in theme templates. Much rather, the #1 use-case is to do this:

drupal_add_js(libraries_get_path('whatever') . '/whatever.js');
Sumeet.Pareek’s picture

Sumeet.Pareek CreditAttribution: Sumeet.Pareek commented

@sun: Thanks for replying.

I never knew folks are using this module to manage PHP libraries. Always thought it is being used for JS/CSS/Flash etc. Hope that explains the bold assumption on my part.

I agree with you on the probable #1 use-case, but fail to see why base_path inclusion should not be a better default choice.

Considering that the module recommends to create the libraries directory at SITE-ROOT/sites/all/libraries (ie within the drupal site structure), inclusion of the base_path is required in all cases (including when passing path to url()) except for PHP libraries.

I would not mind either of the decisions :-)
But would maintain that during site development (http://localhost/project-X/) per majority use-case the default should include the base_path :-p

PS: Though I am following this issue and have email notifs ON, I did not get one :-/

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Title: libraries_get_path() breaks contributed modules when site is installed with RewriteBase » Decide whether $base_path should default to TRUE
Category: bug » task

After opening this for the third time thinking something was actually broken, fixing title and marking "task". There is nothing broken at all here.

Sumeet.Pareek’s picture

Sumeet.Pareek CreditAttribution: Sumeet.Pareek commented
Assigned: Sumeet.Pareek » Unassigned

Thanks for the edits. The new title reflects the discussions better.

I earlier thought the case for $base_path selection to be TRUE was strong.
But after talking to @sun I believe there are some points in favor of it and some not so.
The module maintainer should close this with or without accepting a patch. Either way would be fine I guess.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs review » Closed (won't fix)

Marking won't fix, to get this out of the way. Don't think anything should be changed.