On the status report page, the link to cron.php doesn't work when clean urls are disabled. The link then points to something like http://localhost/~bart/drupal-7/?q=cron.php&cron_key=1bf726cd2d82b83ea03...
The same thing happens in the cron tests. It executes GET to http://localhost/~bart/drupal-7/?q=cron.php
The url is probably generated using l("cron.php") which happens to work when clean urls are enabled, but doesn't work when clean urls are disabled.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | system-cron-296321-7.patch | 2.81 KB | pwolanin |
| #5 | system-cron.patch | 2.6 KB | mfb |
| #1 | cron.patch | 1.97 KB | swentel |
Comments
Comment #1
swentel commentedCan reproduce. Using external = true seems to help a bit. Attached is a patch which fixes the problem and tests pass now, however, I'm not sure if this is the way to go, so a good review is necessary I think :)
Comment #2
keith.smith commentedThis bug is certainly confirmed (I haven't tried it with clean urls on), but it is certainly broken with clean urls off. I'll review the proposed patch a bit later.
Comment #3
keith.smith commentedAs an additional comment, the 'outside' cron link as it is currently coded does work properly with clean urls turned on.
Comment #4
maartenvg commentedTested patch, applies cleanly, and solves the problem of failing cron-tests when clean urls are disabled, and continuous to pass when clean urls are enabled.
I think this a good and clean solution, as the alternative is worse and because we're dealing with an outlier. The alternative being: determining in url() whether the call is to cron.php (or some other existing file being: update.php, install.php and perhaps xmlrpc.php), and automatically assign $external = TRUE. This unnecessarily adds strain to an important function, to make life easier for only a few instances where it would fail.
Besides, we already work around this 'bug' in url(), for example the link to 'update.php' on the module listing page is created by means of $base_url . '/update.php', without using url().
Therefore, I'm for this patch, in this form. As a result I set this to CNR.
Comment #5
mfbPatch in #1 did not work for sites in a subdirectory.
Comment #6
pwolanin commentedpatch in #5 fails to apply.
Comment #7
pwolanin commentedsimple re-roll (maybe was just a whitespace change). Fixes tests and the status page.
Comment #8
dries commentedCommitted to CVS HEAD. Thanks.
Comment #9
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.