This is a sub-part of the larger patch proposed here: http://drupal.org/node/88633

Go ahead- I dare you. Create a new node type and give it the machine-readable name 0 (just the number zero).

This problem occurs because the validation for "required" form values allows the string '0', but in PHP empty('0') == TRUE, so all the checks for empty() (or not empty()) $type->type values don't work.

Attached patch returns a validation error if the $type->type evaluates to empty. This is the simplest thing I can think of that does not break any existing t() strings.

CommentFileSizeAuthor
#6 empty_type_validate_3.diff922 bytespwolanin
#3 empty_type_validate_2.diff969 bytespwolanin
empty_type_validate_1.diff873 bytespwolanin

Comments

Jaza’s picture

Jaza commented
Status: Needs review » Reviewed & tested by the community

This is about the simplest solution that we can implement for catching this uncommon edge case. I think that other possible solutions, such as enforcing a minimum length of 2 chars for node type names, is overdoing it.

RTBC.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Needs work

The error message, "Invalid machine-readable name 0," is not helpful. Why isn't this valid? What can be done to fix it? Why is it 'machine-readable' in the message but the field is labeled 'type'?

pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
StatusFileSize
new empty_type_validate_2.diff969 bytes

Here's an improved error message.

pwolanin’s picture

pwolanin commented
Status: Needs review » Reviewed & tested by the community

since the text seemed to be the only problem, setting back to RTBC

ChrisKennedy’s picture

ChrisKennedy commented
Status: Reviewed & tested by the community » Needs work

The comment should go on its own line imo.

More importantly, the error message is inaccurate: "Please use a type name with more than one character, or a character other than zero."

As far as I can tell there is no requirement that the type be more than one character. I think you're trying to say that it needs to be at least one character, and also cannot be '0'. However, I this message need only occur if the type is zero itself, because #required is already set to TRUE for $form['identity']['type'], so we know that it isn't a truly empty string. See _form_validate().

If I'm right, the error message should be something like: "Please choose a type name other than '0'." This also suggests that the comment should be modified, and you might as well make the if test be === '0' to be clearer.

pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
StatusFileSize
new empty_type_validate_3.diff922 bytes

FYI, in form.inc, the code validating "required" fields in _form_validate() is:

      if ($elements['#required'] && empty($elements['#value']) && $elements['#value'] !== '0') {
        form_error($elements, t('!name field is required.', array('!name' => $elements['#title'])));
      }

revised patch attached for this annoying edge case.

ChrisKennedy’s picture

ChrisKennedy commented
Status: Needs review » Reviewed & tested by the community

Updated patch looks good and works as designed.

I hope in d6 we can remove the lazy empty(), etc. checks throughout core that create these edge cases. That is an issue for another day.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)