user_forms() is now redundant [#1316866]

Hi all,

I was looking at hook_forms() implementations in core and I came across user_forms(). Checking it out, it seems that the callback - user_admin_access_form() - no longer exists in D7 or D8.

It did exist in D6 but I'm guessing that user_forms() is no longer used and can be removed? Unless I have completely misunderstood how hook_forms() works?

user_forms() is also referred to in comments in a few files - such as form.inc .

So basically, I think we need to:

a) remove the user_forms() function from the user module
b) remove all other references to user_forms() within Drupal - in comments basically
c) backport the above to D7 (not sure if we backport this kind of change though?)

I'm happy to do this myself once I have confirmation that I am not completely wrong on this (a possibility given my noobiness).

Thanks,

nicl

Comment	File	Size	Author
#10	1316866-user-forms.patch	2.8 KB	Niklas Fiekas
#10
#6	user_forms_reduntant-1316866-6.patch	2.84 KB	nicl
#6
#5	user_forms_reduntant-1316866-5.patch	10.43 KB	nicl
#5

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

nicl CreditAttribution: nicl commented 21 October 2011 at 12:46

Just to clarify, I did look for whether user_forms() was called anywhere but couldn't find any instances where it is for D7/8. But again, verification would be great!

Comment #2

xjm

she/her

English

CreditAttribution: xjm commented 21 October 2011 at 14:02

Category:

bug

» task

Guess we can call this a cleanup task. It's backportable.

user_forms() would not be called directly since it is a hook implementation. However, since it's not doing anything and the forms it's routing no longer exist, we should get rid of it.

Results are the same in both 7.x and 8.x branches:

[milankovitch:drupal | Fri 08:54:36] $ grep -r "user_forms" *
includes/form.inc: *   search_forms(), and user_forms().
includes/form.inc: *   search_forms(), and user_forms().
includes/form.inc: *   may be found in node_forms(), search_forms(), and user_forms().
includes/form.inc: *   may be found in node_forms(), search_forms(), and user_forms().
modules/user/user.module:function user_forms() {
[milankovitch:drupal | Fri 08:54:41] $ grep -r "user_admin_access_form" *
modules/user/user.module:  $forms['user_admin_access_add_form']['callback'] = 'user_admin_access_form';
modules/user/user.module:  $forms['user_admin_access_edit_form']['callback'] = 'user_admin_access_form';
[milankovitch:drupal | Fri 08:58:38] $ grep -r "user_admin_access_add_form" *
modules/user/user.module:  $forms['user_admin_access_add_form']['callback'] = 'user_admin_access_form';
[milankovitch:drupal | Fri 08:58:44] $ grep -r "user_admin_access_edit_form" *
modules/user/user.module:  $forms['user_admin_access_edit_form']['callback'] = 'user_admin_access_form';

So, the callbacks no longer exist, the base form no longer exists, and we should remove all references to all of them.

Comment #3

nicl CreditAttribution: nicl commented 21 October 2011 at 14:05

Ah yes, of course it won't be called directly. Pretty stupid of me! Thanks xjm, I'll work on a patch this weekend.

Comment #4

xjm

she/her

English

CreditAttribution: xjm commented 21 October 2011 at 14:06

Issue tags:

+Novice, +Needs backport to D7

Tagging.

Comment #5

nicl CreditAttribution: nicl commented 2 December 2011 at 16:49

Status:

Active

» Needs review

File	Size
user_forms_reduntant-1316866-5.patch	10.43 KB

Patch for D8, D7 will follow shortly...

UPDATE: Erm, I included another patch in that, so please ignore.

Comment #6

nicl CreditAttribution: nicl commented 2 December 2011 at 17:18

File	Size
user_forms_reduntant-1316866-6.patch	2.84 KB

Ok, should be right this time...

Comment #7

watbe CreditAttribution: watbe commented 16 January 2012 at 00:22

Status:

Needs review

» Reviewed & tested by the community

Can't find anything wrong with it.

Comment #8

sun

German

Karlsruhe

CreditAttribution: sun commented 24 January 2012 at 22:52

hook_forms() is still an actively used Form API hook.

What matters here is the referenced 'callback' of user_admin_access_form(), which no longer exists since D7.

Hence, this is good to go.

Comment #9

Dries CreditAttribution: Dries commented 24 January 2012 at 22:53

Version:	8.x-dev	» 7.x-dev
Status:	Reviewed & tested by the community	» Needs review

Alright! Committed to 8.x. Thanks.

Comment #10

Niklas Fiekas CreditAttribution: Niklas Fiekas commented 11 February 2012 at 23:59

File	Size
1316866-user-forms.patch	2.8 KB

Rerolled for D7.

Comment #11

xjm

she/her

English

CreditAttribution: xjm commented 12 February 2012 at 20:44

Status:

Needs review

» Reviewed & tested by the community

Backport looks good as well.

@webchick: If you look at this and go "How is this backportable again?" like I just did, see #2. (I suppose if we wanted to be extra-cautious in case some misled person is calling user_forms() directly, we could backport only the documentation fixes...)