link to cron.php broken when clean urls are disabled [#296321]

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
#7
#5	system-cron.patch	2.6 KB	mfb
#5
#1	cron.patch	1.97 KB	swentel
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

swentel CreditAttribution: swentel commented 17 August 2008 at 11:33

Status:

Active

» Needs work

File	Size
cron.patch	1.97 KB

Can 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 CreditAttribution: keith.smith commented 19 August 2008 at 14:40

This 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 CreditAttribution: keith.smith commented 19 August 2008 at 15:06

As an additional comment, the 'outside' cron link as it is currently coded does work properly with clean urls turned on.

Comment #4

maartenvg CreditAttribution: maartenvg commented 9 September 2008 at 07:09

Status:

Needs work

» Needs review

Tested 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.