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)
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)
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 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)
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)
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 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)
commented

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

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Issue summary: View changes
Status: Active » Closed (outdated)

I'll close this issue due to inactivity for 14 years and upcoming EOL of Drupal 7 in January 2025. While the project's version for Drupal 7 will surely keep providing support for security issues and website breaking bug fixes for Drupal 7, we should try to minimize the open issues for Drupal 7 in the queue as much as possible. A big thanks around for the report and all the efforts in here. Feel free to re-open if you can provide a fix or found an additional related problem which increases the issue priority.