Not sure whether this should be filed against form.inc or user.module.

User pages show this:

    * notice: Undefined index: user_profile_category in includes/form.inc on line 987.
    * notice: Undefined index: user_profile_item in includes/form.inc on line 987.
    * notice: Undefined index: user_profile_item in includes/form.inc on line 987.

This is perplexing because the type #user_profile_item is *not* a form system type. It's a themed type defined by user_theme and used in $account->content in the hook_user function:

  if ($type == 'view') {
    $account->content['user_picture'] = array(
      '#value' => theme('user_picture', $account),
      '#weight' => -10,
    );
    if (!isset($account->content['summary'])) {
      $account->content['summary'] = array();
    }
    $account->content['summary'] += array(
      '#type' => 'user_profile_category',
      '#attributes' => array('class' => 'user-member'),
      '#weight' => -5,
      '#title' => t('History'),
    );
    $account->content['summary']['member_for'] =  array(
      '#type' => 'user_profile_item',
      '#title' => t('Member for'),
      '#value' => format_interval(time() - $account->created),
    );
  }

I have no idea where user_profile_item gets passed to the forms system, so I don't have a patch.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

asimmonds’s picture

FileSize
720 bytes

I'm not completely certain if the #type is correct or the if #type is a typo for #theme, substituting #theme doesn't really work properly, coming up with a #children warning from the theme template.

but if #type is correctly used here, then we are using custom form elements so we need a hook _elements implementation.

asimmonds’s picture

Component: user.module » forms system
Status: Active » Needs review
FileSize
526 bytes

Reading the background issues that introduced this code into profile and user modules:
http://drupal.org/node/144397 (Use drupal_render() for profile page display) and
http://drupal.org/node/79227 (use drupal_render() for profile display)

Supposedly hook _elements() isn't required for this usage, as the form code should use the default element attributes if a #type is not defined in a _elements() hook.

Attached is a patch to form.inc/_element_info() to return the default attributes if the #type is not in the cached list of form element defaults.

jbratton’s picture

Status: Needs review » Reviewed & tested by the community

The patch looks good. It has been tested against a fresh cvs install. It looks like this was the way the form.inc/_element_info() function was actually intended to work. RTBC.

AjK’s picture

Hi, as per your comment here: http://drupal.org/node/157849

The only reason I choose not to fix it in the return function was

  1. Your patch changes the return param from a very commonly called API function and although it's right (imho) we're in code freeze and changing any return parameter now may cause problems elsewhere
  2. My patch fixes the cause rather than the effect
  3. Lastly, blatent name dropping this, I discussed this with eaton in #drupal and the conclusion was anything that creates a new #type should define it

I guess the core committers will need to review both pathches and make a decision.

AjK’s picture

I should have made this more clear above but I'd +1 this patch as:

  1. I came to the same conclusion without seeing this patch
  2. I did a grep -r "#type" on the Drupal tree to find all instances of #type (yes, there's a lot!) and I could only find these two in user.module that introcduced new #types that are not handled by system.modules hook_elements()

So, I don't see comitting this patch would break anything else but atthe end of the day it's not my call to commit.

I've of the opinion that both patches should be committed. But I would say that wouldn't I!

Dries’s picture

Is it me or is the proposed patch a bit hack-ish?

AjK’s picture

Which, there are actually two patches here and in some sense they both need to go in.

First, the addition of hook_elements() to user.module is needed because hook_user() defined two new #types to drupal_render() and if they are not defined then _element_info() can return, well, nothing. But worse is that "nothing" in the sense of an array key index is a E_NOTICE warning.

Second, reallistically _element_info() should be allowed to return the way it does as it's capable of returning badly with an E_NOTICE. The second patch fixes that. However, I am of the opinion that it should be left there to generate the E_NOTICE as the real error is in not defining the correct #type with hook_elements(). So although making _element_info() check it's return type shouldn't go in as the E_NOTICE is a pointer to the real problem, being the failure to define #type that you intend to use.

I'm not sure what's hackish about adding hook_elements() to user.module. If it want's to define a new #type for drupal_render() then it should do it "as per the API says, use hook_elements()". If that's hackish then the API is a little broken in that respect.

AjK’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
690 bytes

for what it's worth, here's the patch I did off the other issue which is basically the same solution as an earlier patch on this issue. All it does is use hook_elements() to define the two new #type before hook_user() actually wants to use them. I'll mark my other issue as dup of this one.

I believe this is teh way to solve this rather than forcing _element_info() to always return an array.

Setting to "needs review" as obviously it does. But it's worked for two of us so far :)

asimmonds’s picture

@ajk: I was working off a quote from eaton in a answer to drumm (about a year ago, so it may not actually be valid now):

(drumm) Doesn't #type => dl need an entry in system_elements()?
(eaton) Nope. system_elements() is necessary to provide default values for array properties that aren't specified, and other conveniences, but by itself, #type maps to theme functions.

I read this as a hook_elements() is not required if the default attributes returned by _element_info() are suitable.
But, I agree that we had better not change the API post-freeze. I suppose I should have asked eaton or chx in IRC about this...

AjK’s picture

if that's the case then _element_info() should always return an array, even if it's empty. So that isn't hackish either.

@Dries, the two propsed patches solve the issue and do so in a manner that's consistent with the API so I don't believe them to be hackish. Either:

  • The caller defines the #type ahead of time using hook_elements() it wants to use (uses API to define)
  • The API should return a consistent datatype or the return value should be handled correctly
  • or both

As it stands you can pass a #type to _element_info() that's not in it's page cache. Because _element_info() returns return $cache[$type}; the possibilty for $type not being in the $cache array exists. That's a E_NOTICE violation. So doing return (isset($cache[$type]) ? $cache[$type] : array(); isn't imho hackish but is saying "we always return an array from this API function even if it's empty".

If the quote above is correct and #type doesn't need to be defined ahead of time then the _element_info() function should be altered to always return an array even if empty. So, my question really is "what's hackish about ensuring a function returns a consistent data type when?"

GoofyX’s picture

Using July's 26 snapshot, I get the same notices, but in different lines:

notice: Undefined index: user_profile_category in /home/lourdas/public_html/d6/includes/form.inc on line 1085.
notice: Undefined index: user_profile_item in /home/lourdas/public_html/d6/includes/form.inc on line 1085.
notice: Undefined index: user_profile_item in /home/lourdas/public_html/d6/includes/form.inc on line 1085.
yelvington’s picture

Using the July 30 snapshot, I get the same errors on line 1137.

* notice: Undefined index: user_profile_category in /web/sitename/htdocs/includes/form.inc on line 1137.
* notice: Undefined index: user_profile_item in /web/sitename/htdocs/includes/form.inc on line 1137.

steinmb’s picture

Status: Needs review » Active

I get in:
Clean install of Drupal6 dev 31.7.2007
MySQL: version: 4.1.20
PHP version: 4.3.9

notice: Undefined index: user_profile_category in /includes/form.inc on line 1137.
notice: Undefined index: user_profile_item in /includes/form.inc on line 1137.

AjK’s picture

Status: Active » Needs review

Please don't change the status unless you're reviewing the patches.

dvessel’s picture

Status: Needs review » Reviewed & tested by the community

+1

Works for me. Tested in a user page with all field types. I still get errors but it's not related to this.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)