Use hook_form_FORM_ID_alter() where possible

See #331832: Document hook_form_FORM_ID_alter() for documentation.

Justin, could you share your before/after number comparisons? You said it was "not as great as you'd hoped" but does it buy us something? If so, how much?

i reviewed the code and saw $form_id = NULL

+function update_form_system_modules_alter(&$form, $form_state, $form_id = NULL) {

is there a reasion why to do it, the argument is not there always, there was even a php error when using openid module

there was also some spaces left
PS: whats about left spaces in all drupal core?

sometimes i don't understand it, the patch was there

Ok, fair enough. :)

Committed to HEAD. Thanks!

_openid_user_login_form_alter uses $form_id to distinc between the width of the field.

so there has to be set the $form_id, or it does not work

The code that uses $form_id in _openid_user_login_form_alter is:

  $form['openid_identifier'] = array(
    '#type' => 'textfield',
    '#title' => t('Log in using OpenID'),
    '#size' => ($form_id == 'user_login') ? 58 : 13,
    '#maxlength' => 255,
    '#weight' => -1,
    '#description' => l(t('What is OpenID?'), 'http://openid.net/', array('external' => TRUE)),
  );

What might be a better method is:

    '#size' => $form['name']['#size'] - 2,

Since on user_login, the size is 60, and on user_login_block, the size is 15.

wow there is so much wisdom in drupal community!

is there needed to write a test for this?

Yes, it would be nice to make a couple baseline tests to ensure that the OpenID login works. This is exactly the kind of thing that our testing suite should have found, and will continue not to find until we have test coverage for it.

The fix looks nice and elegant, though. :) But I'm wondering why we need to be -2? Could we make it simple and just use the same width as the user_login form?

i'm just guessing, its because of the openid icon.
But both lines (normal user name and openidname) should have the same width,
is it possible to figure out who wrote this code?

The 'Comment settings' is now completely underneath the 'Save content type' submit button. Weird thing is, adding #weight to the fieldset doesn't affect anything ... not sure if it's because of this patch or not though, just mentioning ...

Followup with comment settings weight in #364484: Comment settings are below the Save content type button.

I have successfully applied the patch from #14 and have manually tested.

Should we create a separate issue to get this particular patch commited? Or is the intent to wait until tests are also built?

I ran the patch and tested it. Worked fine for me. I'd like to apply it before the testing so that the error above can be eliminated:

<div class="form-item" id="edit-openid-identifier-wrapper">
 <label for="edit-openid-identifier">Log in using OpenID: </label>
 <input type="text" maxlength="255" name="openid_identifier" id="edit-openid-identifier" size="58" value="" class="form-text" />
 <div class="description"><a href="http://openid.net/">What is OpenID?</a></div>
</div>

Mike

Sorry, changed the overall title. Just wanted to bring attention to this warning:

Notice: Undefined variable: form_id in _openid_user_login_form_alter() (line 117 of /home/dm7/modules/openid/openid.module).

Grumble, mutter...

Yeah, I guess I can commit this. It's bothering me a lot that I'm committing a bug fix without a test to back up that it's fixed. Just means we'll break things again in the future. :( Hope to see you guys at #251245: openid.module needs tests, as eager to help out there and get those committed. ;)

I can't really see a good reason to remove 2 from the size of the parent field, so I removed this from the patch and committed it. If someone finds an instance where this is break (I didn't in clicking through in Garland), let's fix it in CSS, not in the form definition.

some functions now take &$form_state instead of $form_state due to patches in this issue, yet I can't find anywhere why this was changed and the functions in question don't change anything in $form_state, so is there any reason to pass it as reference?

-> #503892: hook_form_alter() and hook_form_FORM_ID_alter() take $form_state as reference

also extracted the node_type form for upload to upload_form_node_type_form_alter -> http://drupal.org/node/503780#comment-1748732

not sure if this also applies to profiles, but I don't rly see any reason why not

forgot to remove the $form_id from the default profile

default.profile is now standard.profile. Because of this, I considered setting this to "needs work", but given the age of the issue, I'm setting it to fixed as per #22. Please open new issues for specific changes, such as standard_form_alter() to standard_form_install_configure_form_alter(), or re-open this issue if you're including a full sweep of many places.