This patch automatically enables clean URLs during installation using Ajax to check if the server supports it. Dependent on http://drupal.org/node/97213

CommentFileSizeAuthor
#8 install_cleanurls_2.patch4.41 KBChrisKennedy
#7 install_cleanurls_1.patch4.41 KBChrisKennedy
#3 ajax_cleanurls_6.patch4.99 KBChrisKennedy
#2 install_cleanurls_0.patch3.45 KBChrisKennedy
install_cleanurls.patch2.93 KBChrisKennedy

Comments

heine’s picture

heine commented
Version: 5.x-dev » 6.x-dev
ChrisKennedy’s picture

ChrisKennedy commented
StatusFileSize
new install_cleanurls_0.patch3.45 KB

Syncing with HEAD.

ChrisKennedy’s picture

ChrisKennedy commented
Assigned: Unassigned » ChrisKennedy
StatusFileSize
new ajax_cleanurls_6.patch4.99 KB

I noticed two coding style errors in string concatenation.

Anonymous’s picture

Anonymous (not verified) commented

nice. tested and good. +1

something so simple yet again.. damn i need start thinking of these simple things :P

Anonymous’s picture

Anonymous (not verified) commented

look at the code..

should move the global killswitch to a js file.

ChrisKennedy’s picture

ChrisKennedy commented
Status: Needs review » Needs work

Actually the .js file shouldn't even have been included in the patch, as it's actually part of #97213 (which this issue is dependent on). The killswitch can't be moved into the .js file as it's shared among multiple pages. Also, the patch from #3 was the wrong file.

I'll reroll the patch based on #1 once http://drupal.org/node/97213 or something similar is committed.

ChrisKennedy’s picture

ChrisKennedy commented
Status: Needs work » Needs review
StatusFileSize
new install_cleanurls_1.patch4.41 KB

Re-rolled with some minor updates.

ChrisKennedy’s picture

ChrisKennedy commented
StatusFileSize
new install_cleanurls_2.patch4.41 KB

Sorry, a few more minor updates.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

+1 for this functionality.

I'm confused though... why all those checks? Why not have the auto-set thing happen in system_install() or something, so that it only runs once?

ChrisKennedy’s picture

ChrisKennedy commented

Based on my understanding of Drupal, which could very well be in error, the checks will always need to be in there if we use the current testing technique (Ajax request to a Drupal callback). I'm not sure I follow your alternative suggestions - perhaps you can elaborate?

My understanding is as follows: we need to use some method (javascript currently, but we could theoretically use php/curl instead) to request a standard callback within Drupal, accessed via clean URLs. If it succeeds the callback will be executed and should enable clean URLs, and if it fails the callback will never be triggered (obviously). But AFAIK there isn't a clean way to remove the defined callback post-installation or after the check has succeeded.

But I'm certainly no expert at Drupal internals so I might just be overlooking something obvious.

ChrisKennedy’s picture

ChrisKennedy commented

Well, I should add that within the current suggested implementation, rather than doing those checks we also have the option of setting a variable to indicate that installation was completed, and check within the clean URLs install callback that this variable hasn't been set yet.

I considered this possibility back when I initially looked at the functionality, but I didn't see a simple way for the Ajax to be run automatically, after installation was successful, but before a php variable was set indicating that installation was completed. It's very possible that I didn't give this sufficient consideration.

Now that I think about it maybe we could set the variable with the installation timestamp, and then within the callback check that it is within ~5 minutes of the installation time. And it might be a handy variable to define for general system administration purposes (e.g. by putting it in the status report).

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

I'm not up on AJAX, so you'll need to excuse my ignorance. :)

But it's this just a matter of attempting to grab the contents at something like www.example.com/node and making sure it doesn't return a 404? If so, you could do it with straight PHP (drupal_http_request). And it would work for people without JS too.

chx’s picture

chx commented

neh. drupal_http_request was used before and it's a quagmire of problems

dries’s picture

dries commented

The checks are weird indeed. (I'm in a hurry right now -- I'll follow up with more thoughts.)

ChrisKennedy’s picture

ChrisKennedy commented

Any more thoughts on this? I'm ready to improve it if I can get some more concrete suggestions.

dries’s picture

dries commented
Status: Needs review » Needs work

I think we'll want to eliminate some of these checks. There should be a clean way to check whether we're currently installing Drupal (or whether someone is trying to do unwanted things). The cleanest solution might be to use a tri-state variable. It would have the following states:

clean_urls = -1 // haven't tried installing clean URLs yet
clean_urls = 0 // tried installing clean URLs but failed
clean_urls = 1 // tried installing clean URLs and succeeded

Then all you need to do is:

if (variable_get('clean_urls', -1) == -1) {
  ... code to that tries to enable clean URLs ...
  if ($success) {
    variable_set('clean_urls', 1);  
  }
  else {
    variable_set('clean_urls', 0);
  }
}

Also, let's not create a dedicated watchdog category for clean URL tests. Ideally, this should be 100% transparent for the user, so I'd advocate removing the watchdog() calls and failing silently.

Can you give that a try Chris?

ChrisKennedy’s picture

ChrisKennedy commented
Status: Needs work » Closed (duplicate)

Added clean URLs to http://drupal.org/node/141637 so this isn't needed anymore.