Index: modules/user/user.admin.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/user/user.admin.inc,v
retrieving revision 1.91
diff -u -p -r1.91 user.admin.inc
--- modules/user/user.admin.inc	2 Dec 2009 17:04:25 -0000	1.91
+++ modules/user/user.admin.inc	4 Dec 2009 09:05:14 -0000
@@ -761,19 +761,16 @@ function theme_user_admin_permissions($v
 /**
  * Menu callback: administer roles.
  *
+ * @param $role
+ *   Object containing role name and id.
+ *
  * @ingroup forms
  * @see user_admin_role_validate()
  * @see user_admin_role_submit()
  * @see theme_user_admin_new_role()
  */
-function user_admin_role() {
-  $rid = arg(5);
-  if ($rid) {
-    if ($rid == DRUPAL_ANONYMOUS_RID || $rid == DRUPAL_AUTHENTICATED_RID) {
-      drupal_goto('admin/config/people/roles');
-    }
-    // Display the edit role form.
-    $role = db_query('SELECT * FROM {role} WHERE rid = :rid', array(':rid' => $rid))->fetchObject();
+function user_admin_role($form, &$form_state, $role = NULL) {
+  if ($role) {
     $form['name'] = array(
       '#type' => 'textfield',
       '#title' => t('Role name'),
@@ -785,7 +782,7 @@ function user_admin_role() {
     );
     $form['rid'] = array(
       '#type' => 'value',
-      '#value' => $rid,
+      '#value' => $role->rid,
     );
     $form['submit'] = array(
       '#type' => 'submit',
@@ -812,6 +809,9 @@ function user_admin_role() {
   return $form;
 }
 
+/**
+ * Validation function for the user role add and edit form.
+ */
 function user_admin_role_validate($form, &$form_state) {
   if ($form_state['values']['name']) {
     if ($form_state['values']['op'] == t('Save role')) {
@@ -831,19 +831,22 @@ function user_admin_role_validate($form,
   }
 }
 
+/**
+ * Submit function for user role add and edit form.
+ */
 function user_admin_role_submit($form, &$form_state) {
   $role = (object)$form_state['values'];
   if ($form_state['values']['op'] == t('Save role')) {
     user_role_save($role);
-    drupal_set_message(t('The role has been renamed.'));
+    drupal_set_message(t('Renamed role to %name.', array('%name' => $role->name)));
   }
   elseif ($form_state['values']['op'] == t('Delete role')) {
-    user_role_delete($form_state['values']['rid']);
-    drupal_set_message(t('The role has been deleted.'));
+    user_role_delete((int)$form_state['values']['rid']);
+    drupal_set_message(t('Deleted role %name.', array('%name' => $role->name)));
   }
   elseif ($form_state['values']['op'] == t('Add role')) {
     user_role_save($role);
-    drupal_set_message(t('The role has been added.'));
+    drupal_set_message(t('Added new role %name.', array('%name' => $role->name)));
   }
   $form_state['redirect'] = 'admin/config/people/roles';
   return;
Index: modules/user/user.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/user/user.module,v
retrieving revision 1.1087
diff -u -p -r1.1087 user.module
--- modules/user/user.module	1 Dec 2009 16:03:35 -0000	1.1087
+++ modules/user/user.module	4 Dec 2009 09:05:16 -0000
@@ -1464,9 +1464,9 @@ function user_menu() {
     'file' => 'user.admin.inc',
     'weight' => -9,
   );
-  $items['admin/config/people/roles/edit'] = array(
+  $items['admin/config/people/roles/edit/%user_admin_role'] = array(
     'title' => 'Edit role',
-    'page arguments' => array('user_admin_role'),
+    'page arguments' => array('user_admin_role', 5),
     'access arguments' => array('administer permissions'),
     'type' => MENU_CALLBACK,
   );
@@ -1625,6 +1625,22 @@ function user_category_load($uid, &$map,
 }
 
 /**
+ * Returns a role object.
+ *
+ * @param $rid
+ *   The ID of a user role to alter.
+ *
+ * @see user_admin_role()
+ */
+function user_admin_role_load($rid) {
+  if (!is_numeric($rid) || $rid == DRUPAL_ANONYMOUS_RID || $rid == DRUPAL_AUTHENTICATED_RID) {
+    // This will set page not found
+    return FALSE;
+  }
+  return user_role_load($rid);
+}
+
+/**
  * Returns the user id of the currently logged in user.
  */
 function user_uid_optional_to_arg($arg) {
@@ -2376,13 +2392,15 @@ function user_roles($membersonly = FALSE
  *   exists, FALSE otherwise.
  */
 function user_role_load($role) {
-  $field = is_int($role) ? 'rid' : 'name';
+  $field = is_numeric($role) ? 'rid' : 'name';
+  // is_int() never sets $field to 'rid'
   return db_select('role', 'r')
     ->fields('r')
     ->condition($field, $role)
     ->execute()
     ->fetchObject();
 }
+
 /**
  * Save a user role to the database.
  *
@@ -2424,7 +2442,6 @@ function user_role_save($role) {
  */
 function user_role_delete($role) {
   $role = user_role_load($role);
-
   db_delete('role')
     ->condition('rid', $role->rid)
     ->execute();
Index: modules/user/user.test
===================================================================
RCS file: /cvs/drupal/drupal/modules/user/user.test,v
retrieving revision 1.70
diff -u -p -r1.70 user.test
--- modules/user/user.test	1 Dec 2009 22:30:31 -0000	1.70
+++ modules/user/user.test	4 Dec 2009 09:05:19 -0000
@@ -1319,3 +1319,59 @@ class UserEditTestCase extends DrupalWeb
     $this->assertRaw(t('The name %name is already taken.', array('%name' => $edit['name'])));
   }
 }
+
+/**
+ * Test case to test adding, editing and deleting roles.
+ */
+class UserRoleEditTestCase extends DrupalWebTestCase {
+  protected $admin_user;
+  protected $role;
+  protected $rid;
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Role edit',
+      'description' => 'Test role edit page.',
+      'group' => 'User',
+    );
+  }
+
+  function setUp() {
+    parent::setUp();
+    // Create a user.
+    $this->admin_user = $this->drupalCreateUser(array('administer permissions', 'administer users'));
+    $this->drupalLogin($this->admin_user);
+    $this->role = $this->randomName();
+    // Find the new role ID - it must be the maximum.
+    $this->rid = max(array_keys($this->admin_user->roles));
+
+  }
+
+  /**
+   * Test adding a role.
+   */
+  function testAddRole() {
+    $edit = array('name' => $this->role);
+    $this->drupalPost('admin/config/people/roles', $edit, t('Add role'));
+    $this->assertText($this->role, t('Added role name successfully displayed.'));
+  }
+
+  /**
+   * Test editing a role.
+   */
+  function testEditRole() {
+    $this->role = $this->randomName();
+    $edit = array('name' => $this->role);
+    $this->drupalPost("admin/config/people/roles/edit/{$this->rid}", $edit, t('Save role'));
+    $this->assertText($this->role, t('Edited role name successfully displayed.'));
+  }
+
+  /**
+   * Test deleting a role.
+   */
+  function testDeleteRole() {
+    $this->drupalLogin($this->admin_user);
+    $this->drupalPost("admin/config/people/roles/edit/{$this->rid}", array(), t('Delete role'));
+    $this->assertNoRaw("admin/config/people/roles/edit/{$this->rid}", t('Role edit link removed.'));
+  }
+}