Download & Extend

Multi selects in hook_settings are broken

Project:Drupal core
Version:4.7.0
Component:forms system
Category:bug report
Priority:critical
Assigned:Jaza
Status:closed (fixed)

Issue Summary

The system_settings_form_submit() function handles all form values that are an array in the same way (this includes #radios, #checkboxes, and multi- #select). It checks if they're set, and if so, it calls array_keys() to get just the keys of that array. This works fine for #radios and #checkboxes, where the keys that it's dealing with are the user-submitted values, and the values themselves are either duplicates of the keys, or useless nothing. But for multi- #select, the keys are a meaningless sequence from 0 to (count-1), and the values are the user-submitted data.

I've decided that the problem is not that the way system_settings_form_submit() handles it is incorrect. The problem is that multi-selects are not getting their submitted values processed properly by the FAPI. The expand_checkboxes() and expand_radios() seem to make it work for those two elements, but multi-selects have no such function, and so they miss out - they never get the keys of their submitted values set.

Attached patch fixes this, by making the submitted values array for multi-selects consistent with that for checkboxes and radios. I think that this will make it easier for module developers to work with multi-selects in general, not just in hook_settings(). I couldn't actually find any multi-selects that are being used in a hook_settings() implementation, but I am using such an element in one of my modules, and that's where I discovered the problem.

This patch does not break existing multi-selects - thoroughly tested with the multi-select used for the 'node add/edit' and the 'add/edit term' forms in the taxonomy module, they still work fine. I can't see how any modules will be affected by this patch, unless they're using the keys of a multi-select array, which they really shouldn't be, since those keys are currently quite meaningless. I also think that this is a bug, and that it therefore applies to 4.7 as well as to HEAD.

AttachmentSizeStatusTest resultOperations
multiselect.patch1.16 KBIgnored: Check issue status.NoneNone

Comments

#1

Priority:normal» critical
Status:needs review» reviewed & tested by the community

Updated patch to use drupal_map_assoc() for setting the array keys and values - this function does the same thing as the loop that was in the patch before. Also bumping this issue up to critical, and setting RTBC since I found no problems with it.

AttachmentSizeStatusTest resultOperations
multiselect_0.patch1.09 KBIgnored: Check issue status.NoneNone

#2

OK. It's a bit weird bug, but the fix is OK.

#3

Status:reviewed & tested by the community» fixed

Committed to HEAD.

#4

also to 4.7

#5

Status:fixed» closed (fixed)
nobody click here