| Project: | ProjectPier |
| Version: | 0.8.5.0-Beta1 |
| Component: | User interface |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Tim |
| Status: | patch - code needs work |
It would be nice to have clean URLs instead of index.php?c=controller&a=action¶m1=value1&active_project=2#32
I will attach a patch that still need some work but I would appreciate some testing done (it can break things easily...).
It will create URL in the form of
http://server/controllername/action/?params
or http://server/projectID/controller/action/?params
The position of project ID can be changed if you feel it would make more sense the other way. Some discussion about that on the dev mailing list.
To make it work (works only with Apache!)
- make sure that the mod_rewrite module is loaded in Apache
- apply the patch as you would for any other patch
- add the following line to config/config.php:
<?php
define('PRETTY_URL', true);
?>Make sure that mod_rewrite is loaded, otherwise Apache won't be happy with the .htaccess file that is now modified.
Also, if it doesn't work, you can set the variable PRETTY_URL to false. If you delete the line completely, on my machine, php evaluates PRETTY_URL to true by default...
Here is the patch.
Well, I forgot something important:
you need to modify .htaccess so that the rewrite rules are ok according to your installation.
The patch should write .htaccess with these two last lines:
RewriteRule ^([0-9]+)/([a-zA-z0-9]+)/([a-zA-z0-9]+)?/?$ /PHP/PP_pretty_url/index.php?c=$2&a=$3&active_project=$1 [QSA]
RewriteRule ^([a-zA-z0-9]+)/([a-zA-z0-9]+)?/?$ /PHP/PP_pretty_url/index.php?c=$1&a=$2 [QSA]
you will need to change /PHP/PP_pretty_url/ to whatever your PP installation is.
Here is a patch for review.
Changes since previous version:
* PRETTY_URL becomes CLEAN_URL
* the clean URLs include the Object Id if present: http://server/13/message/view/4/ to view the message of ID 4 in Project of ID 13
* the installation includes the ability to select or not the clean URL. That means no fiddling with the .htaccess or config.php files.
* the .htaccess file works even if the module isn't loaded, which means only one .htaccess file for all. And it's very easy to toggle the option through config.php afterwards.
Please have a look and let me know if you find any problems with it.
NB: to test it on an existing installation, you need to apply the patch and then modify .htaccess to match your base URL (under RewriteBase) and config.php to add
<?phpdefine('CLEAN_URL', true);
?>
I saw that I forgot to remove some stuff in the installation step 4 and that the search was not included.
I'll put a patch in the next couple of days that fixes that but you can still test this one for now ;-)
Here is the patch that I talked about.
Changes since 0.9:
- revert to acInstallation.class.php (a patch to move to ppInstallation.class.php will be created later)
- clean the final installation step (I had forgotten some superfluous printing...)
- made the search function use the clean URLs too.
If you have any comments on the .htaccess file, I'm eager to hear from you. I'm no expert in it, so there might be some more efficient way.
Anybody to test it? ;-)
Tim
We've been using the patch in my company for a couple of months with no problem. However, I would like somebody to review the code and test it. Thanks!
My only comment is that it seems that most calls to get_url() end in 'null, null, false'. I think that if the default on the last argument to get_url() was changed to 'false', then a large number of one-line get_url() calls in the patch could be eliminated, or at least shortened.
Thoughts?
That's silly but why didn't I think about that? :)
I guess it would need to be checked and see if it would create problems somewhere else but I'm not sure why it would...
After further review, my suggestion does create problems. :) However, I have reviewed and installed this patch against the latest SVN and it applies cleanly, although there is a file or two where it has to use a 'fuzz factor'.
Also, I am only able to get this to work if I put my '.htaccess' file in the /public/ directory. Perhaps my Apache settings are incorrect, but I've looked over them multiple times now and the virtual host config seems to be correct to me...
I don't really understand why you would need to put it in /public/... it doesn't seem to make sense since the redirections are made from the root...
I'd like to find the time to try the patch with other servers than Apache (even though I believe Apache is the only supported one) and I'd like to make sure that the plugin patch is fine with it too.
But, overall, you didn't see any problem with the patch then?
I saw no problems with the patch, and it functions perfectly for me after moving the .htaccess to the public/ folder. The patch will apply cleanly to SVN (r162) with a "fuzz" warning on one file that can be safely ignored.
Side note: I'm now almost positive that the .htaccess location is simply a by-product of my Apache configuration... I'm paranoid and I've got Apache locked down as much as possible. The "public/" folder is the DocumentRoot of my virtual host, and I'm pretty sure that I disabled "AllowOverride" for all directories unless explicitly enabled, which is why it only works when the .htaccess is in my "public/" folder (where I have enabled AllowOverride).
Again, I've verified that this patch is good and functions properly. I think this is a good candidate for 0.8.5, so I'm marking it "ready for commit".
"I think this is a good candidate for 0.8.5, so I'm marking it "ready for commit"."
Yay! :)
I couldn't get past the installation. I fill out the "Complete the Installation" screen where you assign an administrator to your new installation and when I submit the form I get a 404 Object Not Found error from Apache. It's trying to access the url: http://vbox1/apache3/access/complete_installation/
I'm using XAMPP on WinXP Pro using the following software versions (from my 404 error page): Apache/2.2.11 (Win32) DAV/2 mod_ssl/2.2.11 OpenSSL/0.9.8i mod_autoindex_color PHP/5.2.8
I'm not going to change the status right now, I'm going to do more testing tomorrow.
Any ideas?
I found a very small bug in this patch. It does not affect functionality of the patch, but it does cause some unnecessary error messages to be printed to the ProjectPier log file.
The error is in line 113 of application/functions.php, the 'if (trim($active_project) <> '')' line. It is possible that active_project is not defined at this point: see the 'if {} elseif {}' in the immediately preceding lines.
The attached patch fixes the error message by testing isset($active_project).
Patch fails on file: public/install/installation/templates/layout.php
Nothing to worry about, from what I could see failure is due to the difference in the lines that have the PP license links.
Patch also introduces a wrong code practice, where a named array is accessed without the quotes: $_SERVER[PHP_SELF] instead of $_SERVER['PHP_SELF']. Probably just a minor oversight.
I'll do some more testing and try to provide a unified patch, with the undefined_variable correction applied.
I am curious as to why this functionality was not included in 0.8.6. I've reviewed the comments and there was no show-stopper bugs. I believe the status was incorrectly changed in comment 16--the user probably was attempting to apply the patch to a different SVN version.
It was never included because it relies on .htaccess and mod_rewrite. Those are Apache specific things and usually an issue on shared servers. Of course, it can be turned off but once it is put in people may want to use it and start posting requests for help. I think it is fair to say it is a political decision.
I also think it needs more work and testing, especially with accented characters: http://htmlblog.net/seo-friendly-url-in-php/. I don't think we have accented characters now in our urls but it might change when we are going to use SEO urls.
After posting I finally found the article I was searching for: http://pumka.net/2009/12/30/rewriting-for-seo-friendly-urls-htaccess-or-php. I think the "Parsing REQUEST_URI by PHP code" section up to "But how to get" describes an approach that is worth a try. There would be no rewrite involved of the application.
URLs would become something like http://somedomain.com/index.php/dashboard/12
Another interesting article: http://php.dzone.com/news/generate-search-engine-friendl