theme_textarea() expects $element['#value'] to be set

Subscribing

also met this bug

This should be an easy one to fix -- just a simple one liner would suffice.

$element['#value'] = isset($element['#value']) ? $element['#value'] : '';

Tagging as novice.

I tried to fix it. It is actually my first commit to Drupal so I hope you don't mind if there are errors.

Sorry I forgot to change the status.

Bladedu: Great! I'm changing the issue status to "Needs Review". This triggers an automated system test and lets others know that they should review your patch.

It failed miserably... I think (but I'm not sure) depends on the patch filename, so I'm commiting it again without spaces.

Ohh god! I'm removing also the # from the filename... I feel like a spammer right now XD.

Hey guys I'm really a n00b! I'm setting the status to need review

+++ b/includes/form.incundefined
@@ -3709,12 +3709,13 @@ function theme_form($variables) {
- *     #placeholder, #required, #attributes
+ *     #placeholder, #required, #attributes, #value

I think we should probably keep #value in the docstring since it is used, but isn't required (just like #rows, #cols, etc).

Edit: I totally misread the patch. However, there is already #value on the line before, so we don't need to add it again.

Powered by Dreditor.

I reviewed this patch, applied it to a clean D8 install, then added the following to the bottom of the page.tpl.php as a means of testing:

$item['mynewtextarea'] = array(
  '#type'=>'textarea',
);
print drupal_render($item);

This worked, produced a nice textarea, without error. I am not sure how to write a test for this (or anything) having never written one, but this test seems to prove that the patch addresses the issue.

Looks good to me!

Pat

Can we fix the docstring as in #12?

Actually my patch adds the #value in the docstring, or am I missing something?

Oh, I totally misread your patch! Isn't there a #value on the previous line already?

No, I added it... the documentation was also wrong.

+++ b/includes/form.incundefined
@@ -3709,12 +3709,13 @@ function theme_form($variables) {
  *     Properties used: #title, #value, #description, #rows, #cols,
- *     #placeholder, #required, #attributes
+ *     #placeholder, #required, #attributes, #value

Now there is #value twice.

6 days to next Drupal core point release.

You were so right. I recreated the patch keeping the function documentation as is.

Looks good, tested it locally

I will check your patch, Bladedu.
I was already working on a patch with the same result, I think.
This is my first ever core patch. Would it be good?

@bassthiam, you're patch is exactly the same, so it should be fine. Let's wait for feedback from the testbot and then we can RTBC it again.

Wow, talk about derailment... I'm reuploading #20 which was marked RTBC in #21.

Sorry, what's the use-case of a textarea that doesn't go through Form API?

Second, do we really want to make all form element theme functions compatible for rendering elements without Form API? If so, did you test the other elements?

Third, speaking of tests, if this is desired and expected functionality, then it has to be covered by tests.

Lastly, the ternary operator value assignment that assigns the same value if it already exists is a bit unclean. More readable would be:

  if (!isset($element['#value']) {
    $element['#value'] = '';
  }

but since the actual issue is only about preventing a PHP notice, the same can be achieved using:

  $element += array('#value' => '');

Shrug.

Second, do we really want to make all form element theme functions compatible for rendering elements without Form API? If so, did you test the other elements?

Well, the only similar element to test would be theme_textfield which does not throw any error if #value is omitted. In retrospect, I think you are correct that the original author probably didn't understand the form api. Therefore I'm content marking this as won't fix.

Lastly, the ternary operator value assignment that assigns the same value if it already exists is a bit unclean.

Agreed, but it is peppered throughout Drupal already (just try running find . -type f | xargs egrep "\\\$(.+) = isset\(\\\$\\1\) \\? \\\$\\1 :"). The final $element += array('#value' => ''); is of course the cleanest.