Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The user registration form looks different depending on the setting of User-configurable time zones (see attached screenshot). If this setting is enabled, the username and e-mail are enclosed in a fieldset; if the setting is disabled, they are not.
It is probably related to this part of user_register():
// Remove form_group around default fields if there are no other groups.
if (!$extra) {
foreach (array('name', 'mail', 'pass', 'status', 'roles', 'notify') as $key) {
if (isset($form['account'][$key])) {
$form[$key] = $form['account'][$key];
}
}
unset($form['account']);
}
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | user_register-1.patch | 2.79 KB | c960657 |
| #4 | user-460706-3.patch | 1.34 KB | wulff |
| #1 | user-460706-remove-fieldset.patch | 1005 bytes | wulff |
| screenshot.png | 47.8 KB | c960657 | |
Comments
Comment #1
wulff CreditAttribution: wulff commentedIt makes sense to hide the fieldset if $extra only contains hidden fields.
See attached patch.
Comment #3
wulff CreditAttribution: wulff commentedSeems one of the tests in user.test might need some adjusting. I'll look into this when simpletest plays nice again.
Comment #4
wulff CreditAttribution: wulff commentedOn second thought, let's try this.
Comment #5
c960657 CreditAttribution: c960657 commentedThese two comments overlap. I suggest you remove the reference to "form_groups" – AFAIK this is history.
When investigating the code in question I noticed some other issues related to this code. These are outside the scope of this issue, so feel free to ignore them :-) But here goes:
- You may want to incorporate the patch for #227690: hook_user('register') implementations can't inject fields into account (the main one) fieldset.
- Instead of moving the controls, it may be better to simply change $form['account']['#type'] from 'fieldset' to 'markup'. This results in the same layout effect (I think), but it preserves the form structure, and this makes life easier for modules altering the form using hook_form_alter.
Comment #6
Dries CreditAttribution: Dries commentedIn the code comments, we'll also want to explain _why_ we're doing what we're doing, not only _what_ we're doing.
Comment #7
catchNeeds work based on those comments.
Comment #8
c960657 CreditAttribution: c960657 commentedSlightly different approach. Using the idea from the patch in #227690: hook_user('register') implementations can't inject fields into account (the main one) fieldset it is now possible to inject fields into $form['account']. Modules should do that if they have invisible fields or don't want to trigger the layout change.
Comment #9
wulff CreditAttribution: wulff commentedThis seems like a sensible solution.
Tested and seems to work as expected.
Comment #10
Dries CreditAttribution: Dries commentedWould be great to get some screenshots of this.
Comment #11
catchDries, There's a screenshot in the opening post, I assume the idea is that there's zero change with the patch.
Comment #13
lilou CreditAttribution: lilou commentedSetting to previous status - testbot was broken (failed to install).
Comment #15
c960657 CreditAttribution: c960657 commentedSetting to previous status - testbot was broken (failed to install).
Comment #17
c960657 CreditAttribution: c960657 commentedSetting to previous status - testbot was broken (failed to install).
Comment #18
webchickThis change looks good to me.
@Dries: Basically, this is fixing a visual regression. In Drupal 6 and below, if "Account information" is the only profile category available, you don't get a fieldset on user/register. But currently, you do, because the user timezone field is not contained within the rest of the account information variables. This fixes that, and also cleans up the logic a bit to remove a bunch of hard-coded assumptions about what the "Account information" category ought to contain. +1.
Committed to HEAD!
Comment #20
c960657 CreditAttribution: c960657 commentedThis has regressed once again: #637712: Fieldset is back in user registration form