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

Comments

Uwe Hermann’s picture

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 commented
Status: Active » Needs review
StatusFileSize
new 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 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 commented
StatusFileSize
new 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 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
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 commented

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

edmund.kwok’s picture

edmund.kwok commented
Status: Fixed » Reviewed & tested by the community
StatusFileSize
new 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 commented
StatusFileSize
new 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 commented
Status: Reviewed & tested by the community » Fixed

backported

Anonymous’s picture

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