I think that the checkbox logic needs to be put into one place and hidden behind a function.

yonas # find domain | xargs grep 'Cannot pass zero in checkboxes'
domain/domain_content/domain_content.module:        // Cannot pass zero in checkboxes, so these are converted from -1.
domain/domain_content/domain_content.module:    // Cannot pass zero in checkboxes.
domain/domain_content/domain_content.admin.inc:      // Cannot pass zero in checkboxes, so these are converted from -1.
domain/domain.module:    // Cannot pass zero in checkboxes.
domain/domain.module:      // Cannot pass zero in checkboxes.

In other words, that should only show up once. I've hit problems with checkboxes a few times already while using other domain modules.

Patches welcome. The 'cannot pass zero in checkboxes' is a core limitation that we have to work around, and the new form style (which allows separate formats) hasn't been consolidated. That should be spun off as a separate issue. The problem there is reconciling the form names and a few other minor discrepancies among each usage.

-- by agentrickard

Comments

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

It's in many more places than this. See also Domain Views and Domain CTools.

This might have to wait for the D7 release, in which core will have fixed the underlying issue.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

Or do you simply want a -1 > 0 converter function? I am thinking about standardizing this type of construct:

  // Get the display format of the form element.
  $format = domain_select_format();
  foreach (domain_domains() as $data) {
    // Cannot pass zero in checkboxes.
    ($data['domain_id'] == 0) ? $key = -1 : $key = $data['domain_id'];
    // The domain must be valid.
    if ($data['valid'] || user_access('access inactive domains')) {
      // Checkboxes must be filtered, select listts should not.
      $options[$key] = empty($format) ? check_plain($data['sitename']) : $data['sitename'];
    }
  }
  $form['domain_settings']['domain_id'] = array(
    '#type' => empty($format) ? 'radios' : 'select',
    '#title' => t('Save settings for'),
    '#options' => $options,
    '#required' => TRUE,
    '#description' => t('Select the domain to which these settings apply.'),
    '#default_value' => $default,
  );
  if ($format) {
    $form['domain_settings']['domain_id']['#multiple'] = FALSE;
    $form['domain_settings']['domain_id']['#size'] = count($options) > 10 ? 10 : count($options);
  }
fizk’s picture

fizk CreditAttribution: fizk commented

When D7 core fixes the underlying issue, will it be backported to D6?

What do you mean by standardizing the construct? Basically we should put that repeating block of logic/code into one function, and have other modules update accordingly. Also, no one else should know about this -1 <==> 0 issue, just one function.

Other modules can always build their own form to display domains however they wish, but if they simply want the "default" selection form, it should be as easy as calling domain_content_add_form_widget(&$form);

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

Right, I just haven't gotten around to that. But that is too simple, because the name of the $form element array differs, so you would have to pass that in some way, or do:

$form['myform']['domains'] = domain_form_widget($form, $default_value);

Though I don't know if you need to pass $form here.

That doesn't solve the conversion problem on submit, either, which is why I haven't bothered.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
Version: 6.x-2.1 » 7.x-2.x-dev

This has to be API cleanup for D7.

Use http://api.drupal.org/api/function/hook_element_info/7

fizk’s picture

fizk CreditAttribution: fizk commented

What's wrong with http://api.drupal.org/api/function/hook_elements/6 in D6?

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

D6 version is closed to API changes and bugfix only. See #781422: D7 Sprint roadmap.