I noticed the following. When you start editing existing role and you enter empty string, the result is not what is expected. Insted of geeting the error and edit form, you get an error and add form.

CommentFileSizeAuthor
#9 empty_role_error_2_47.patch1.87 KBedmund.kwok
#8 empty_role_error_47.patch2.74 KBedmund.kwok
#4 empty_role_error_2.patch1.65 KBedmund.kwok
#2 empty_role_error.patch1.38 KBedmund.kwok
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Uwe Hermann’s picture

Uwe Hermann CreditAttribution: Uwe Hermann commented
Version: 4.7.0-rc2 » x.y.z

Confirmed in CVS HEAD. I'll set this to critical, feel free to lower the severity if you don't think it's critical.

edmund.kwok’s picture

edmund.kwok CreditAttribution: edmund.kwok commented
Status: Active » Needs review
FileSize
empty_role_error.patch1.38 KB

Patch checks to display role edit form in a seperate if...else statement rather than at the end of the operation check.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Needs review » Needs work

Also, the "Role name"-textfield is required, and should have the "'#required' => TRUE" attribute. Maybe that can be fixed too.

Good job, guys.

edmund.kwok’s picture

edmund.kwok CreditAttribution: edmund.kwok commented
FileSize
empty_role_error_2.patch1.65 KB

Added the #required attribute to the role name textfield as suggested by Dries.

edmund.kwok’s picture

edmund.kwok CreditAttribution: edmund.kwok commented
Status: Needs work » Needs review

Added the #required attribute to the role name textfield as suggested by Dries.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Needs review » Fixed

Committed to HEAD.

While testing this is noticed that there is no protection against duplicate role names. I assume this is a preexisting problem and should be covered by a separate patch.

edmund.kwok’s picture

edmund.kwok CreditAttribution: edmund.kwok commented

Duplicate role name problem is addressed in http://drupal.org/node/41293.

edmund.kwok’s picture

edmund.kwok CreditAttribution: edmund.kwok commented
Status: Fixed » Reviewed & tested by the community
FileSize
empty_role_error_47.patch2.74 KB

Backported to 4.7 with a slight addition to 'theme_user_admin_new_role'.

edmund.kwok’s picture

edmund.kwok CreditAttribution: edmund.kwok commented
FileSize
empty_role_error_2_47.patch1.87 KB

Updated patch for 4.7. This time no change in 'theme_user_admin_new_role'.

Gerhard Killesreiter’s picture

Gerhard Killesreiter CreditAttribution: Gerhard Killesreiter commented
Status: Reviewed & tested by the community » Fixed

backported

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)