While doing a test translation run on the generic Drupal strings (which are not extracted well still), I found that PHP module has some outdated and insecure explanations on how one should do PHP code.

- "register_globals is now set to off" suggested that this is a recent change, while it is not
- the help text suggested using _GET and _POST directly for form data handling, which is insecure so I replaced it with a suggestion to look at the Form API
- direct usage of the username is better replaced with using a placeholder (although the username can only contain a restricted amount of chars, this is better for education purposes also)
- how the actual example block is added was not explained

Patch attached, please review. "Unfortunately" I was unable to resist the temptation to break apart this big block of text to smaller chunks, just as it is done at every other help text, so the patch is actually bigger then the actual number of textual changes.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Status: Active » Needs review
FileSize
5.15 KB

Doh, just realized a coding style error introduced with my patch and a mention of "sidebar boxes" at the end from the original text, which is obviously not a correct term anymore ("sidebar blocks" would not be correct either). Fixed these.

Gerhard Killesreiter’s picture

" It will be executed when the page is viewed and dynamically embedded into the page. "

Should probably be:

" It will be executed when the page is viewed and any output dynamically embedded into the page. "

Also, let me state that I really dislike PHP blocks/pages. They have on occassion been the cause of a failed Drupal upgrade and are hard to track don if you didn't set up the site yourself.

I usually recommend that people put the PHP code into a function in template.php or (better) a site specific module. If you go with a module you can then directly crate a block hook and save yourslf some troubles.

Maybe this advice could be integrated.

keith.smith’s picture

Assigned: Unassigned » keith.smith

There is some help text that has recently been committed for other parts of the PHP module that can be reused here, for consistency's sake. I'll post an updated patch later today; assigning to myself in the interim.

Gábor Hojtsy’s picture

I also noticed that the PHP module removal notice is actually a bit misleading. Depending on the default input format assigned to the nodes once the PHP module got removed, the PHP code might get visible or might get stripped (ie. could result in empty nodes). Also not all content submitted with the PHP input format are affected but only those which actually used PHP code, which might be a nitpick but still.

keith.smith’s picture

Assigned: keith.smith » Unassigned
FileSize
5.88 KB

Attached is a patch that reuses some of the language in the PHP Filter help page committed earlier, but attempts to communicate all of the items found in the previous patch (#1), along with the comments in #2 and #4.

:) Please review.

cburschka’s picture

Could the relevant text link to the Form API on api.drupal.org? Given that the users who read this are likely to have never been to api.drupal.org before, this would save quite some effort in finding the API.

keith.smith’s picture

Good idea. I had intended to do that but zoned out apparently, instead.

The attached patch contains a link to http://api.drupal.org/api/group/form/6; is this the link you had in mind?

cburschka’s picture

Yeah, that's probably the best place to start for the form API. Well, I have nothing more to add; the patch looks good now.

keith.smith’s picture

FileSize
6.03 KB

Changed a the site to your site. Otherwise, exactly the same.

Dries’s picture

Status: Needs review » Fixed

I read the changes and I agree that they are much-needed. It's sort of bizarre that this went unnoticed that long. I guess no one reads documentation (neither do I ;)).

I think this is RTBC so I committed it to CVS HEAD. Thanks folks.

Gábor Hojtsy’s picture

Dries, translators read these texts and even translate them. Some of them have no knowledge of Drupal programming or realize too late that such changes need to be made, as they do translation after release.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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