I'm working on a project where for legacy reasons, the /js path is unavailable (configured in apache to point elsewhere). It would be nice to be able to use a different path for our lightweight callbacks. Indeed, we're actually trying to hit it from Varnish rather than client side JS, so 'js' isn't even the most semantically appropriate path!

So, here's a patch to allow overriding of 'js' to some other string by way of setting a variable (via $conf in settings.php). I didn't put a UI on it, for several reasons:

1. A form setting the variable in the DB isn't really good enough, as it needs to be around in js_execute_callback() where the DB variables might not have been loaded up.
2. It's a pretty weird edge case, and using this setting will prevent other contrib integrations with the JS module from working properly. Assuming the other modules integrating have implemented a hook_menu() fallback then nothing will break - they'll just loose the reduced-bootstrap speed boost. In our case, they weren't working anyway so it's no loss.

Totally understand if this doesn't get committed - but this issue gives me somewhere to stick my patch for use with Drush make :)

CommentFileSizeAuthor
#9 allow_callback_path-2689291-8.patch3.34 KBmarkhalliwell
#3 js_callback_path_override-2689291-3.patch1.71 KBstevetweeddale
#2 2689291_js_callback_path_override.patch0 bytesstevetweeddale
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stevetweeddale created an issue. See original summary.

stevetweeddale’s picture

stevetweeddale CreditAttribution: stevetweeddale at ComputerMinds commented
Issue summary: View changes
FileSize
2689291_js_callback_path_override.patch0 bytes
stevetweeddale’s picture

stevetweeddale CreditAttribution: stevetweeddale at ComputerMinds commented
FileSize
js_callback_path_override-2689291-3.patch1.71 KB

*Ahem*, and an actual patch instead of an empty file…

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented
Title: Allow callback path prefix override » Allow callback path prefix to be configurable
Version: 7.x-1.0 » 7.x-2.x-dev
Assigned: stevetweeddale » Unassigned
Status: Active » Needs work

Thanks for the patch! I'm tempted to say that this issue is a non-starter for the following reasons:

  1. 7.x-1.x cannot have any new features, this should be against 7.x-2.x
  2. This would require all callbacks to specify, at a minimum, a DRUPAL_BOOTSTRAP_VARIABLES bootstrap level. variable_get is not useable with the current minimum bootstrap level, DRUPAL_BOOTSTRAP_DATABASE.
  3. The Apache rewrite rules are just a suggestion, introducing this feature wouldn't actually work until the server apache file is changed to match. This alone makes me question if using a variable at all is even useful?

I won't close it just yet, but will set to CNW instead.

stevetweeddale’s picture

stevetweeddale CreditAttribution: stevetweeddale at ComputerMinds commented

Hi Mark,

As I say, this is definitely an edge case, but one that we had to work around, so figured a patch on D.O. would at least be worthwhile. So for the sake of others who might read this (including myself in a years time), here's some response to your points:

1. IIRC, we struggled to get the 2.x version of the module working on our site, because of broken integrations with existing modules. They were aimed at the 1.x version, and as we didn't need any kind of permissions handling, that was fine for our purposes.
2. I understand the bootstrap issue; you'll note that in the issue description I say the patch allows a user to set the variable via $conf in settings.php, because that gets read in right at the very first bootstrap level. This is why I didn't write a UI, because saving it to the DB wouldn't be sufficient, for the reasons you give.
3. Honestly, it's been so long since I've worked on anything where traffic has hit something other than index.php that I'd forgotten that without the .htaccess it would fallback to PHP's default route handling… I'd also sort of assumed that Drupals default .htaccess would route *all* requests to index.php regardless of any conveniently named .php files sat around in the codebase. So my (wrong) assumption was that if the .htaccess suggested by this module wasn't in place then requests would fallback to index.php, and so doing likewise in my case was fine. But you're totally right, php will route it to /js to js.php without any custom .htaccess stuff. As such, it seems renaming js.php would have been a quicker fix in our case, with similar outcomes.

Just out of interest, what advantage then does the suggested .htaccess even provide?

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented
Status: Needs work » Closed (won't fix)

Just out of interest, what advantage then does the suggested .htaccess even provide?

It's necessary to capture a request and route it through this module's js.php file rather than index.php. There's really no way getting around that TBH.

Unfortunately, after reviewing this patch/issue again, I'm still not seeing the benefit of complicating matters more.

All this $conf variable does is modify the .htaccess helper output to indicate what should be placed in that file.

If you're already modifying .htaccess with a custom path that points to js.php, then just put an additional comment above it that explains why you have to rename the /js/ prefix.

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented
Status: Closed (won't fix) » Needs work

Meh, I spoke too soon. I forgot the hardcoded bit in js.php.

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented
Status: Needs work » Fixed
FileSize
allow_callback_path-2689291-8.patch3.34 KB

Here's the 7.x-2.x patch that was committed.

Status: Fixed » Closed (fixed)

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