Index: modules/user/user.admin.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/user/user.admin.inc,v
retrieving revision 1.101
diff -u -r1.101 user.admin.inc
--- modules/user/user.admin.inc	7 Mar 2010 06:49:10 -0000	1.101
+++ modules/user/user.admin.inc	18 Mar 2010 16:43:55 -0000
@@ -766,19 +766,18 @@
 /**
  * Menu callback: administer roles.
  *
+ * @param $role
+ *   A user role object as returned from user_role_load().
+ *
  * @ingroup forms
+ * #see user_role_load()
  * @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/people/permissions/roles');
-    }
+function user_admin_role($form, &$form_state, $role = NULL) {
+  if (!empty($role)) {
     // Display the edit role form.
-    $role = db_query('SELECT * FROM {role} WHERE rid = :rid', array(':rid' => $rid))->fetchObject();
     $form['name'] = array(
       '#type' => 'textfield',
       '#title' => t('Role name'),
@@ -790,7 +789,7 @@
     );
     $form['rid'] = array(
       '#type' => 'value',
-      '#value' => $rid,
+      '#value' => $role->rid,
     );
     $form['actions'] = array('#type' => 'container', '#attributes' => array('class' => array('form-actions')));
     $form['actions']['submit'] = array(
@@ -818,16 +817,19 @@
   return $form;
 }
 
+/**
+ * Form validation handler for user_admin_role() form.
+ */
 function user_admin_role_validate($form, &$form_state) {
-  if ($form_state['values']['name']) {
+  if (!empty($form_state['values']['name'])) {
     if ($form_state['values']['op'] == t('Save role')) {
-      $role = user_role_load($form_state['values']['name']);
+      $role = user_role_load_by_name($form_state['values']['name']);
       if ($role && $role->rid != $form_state['values']['rid']) {
         form_set_error('name', t('The role name %name already exists. Choose another role name.', array('%name' => $form_state['values']['name'])));
       }
     }
     elseif ($form_state['values']['op'] == t('Add role')) {
-      if (user_role_load($form_state['values']['name'])) {
+      if (user_role_load_by_name($form_state['values']['name'])) {
         form_set_error('name', t('The role name %name already exists. Choose another role name.', array('%name' => $form_state['values']['name'])));
       }
     }
@@ -844,7 +846,7 @@
     drupal_set_message(t('The role has been renamed.'));
   }
   elseif ($form_state['values']['op'] == t('Delete role')) {
-    user_role_delete($form_state['values']['rid']);
+    user_role_delete((int)$form_state['values']['rid']);
     drupal_set_message(t('The role has been deleted.'));
   }
   elseif ($form_state['values']['op'] == t('Add role')) {
Index: modules/user/user.test
===================================================================
RCS file: /cvs/drupal/drupal/modules/user/user.test,v
retrieving revision 1.86
diff -u -r1.86 user.test
--- modules/user/user.test	7 Mar 2010 18:46:55 -0000	1.86
+++ modules/user/user.test	18 Mar 2010 16:44:00 -0000
@@ -1456,4 +1456,56 @@
     $account->name = $edit['name'];
     $this->drupalLogin($account);
   }
+}
+
+/**
+ * Test case to test adding, editing and deleting roles.
+ */
+class UserRoleAdminTestCase extends DrupalWebTestCase {
+
+  public static function getInfo() {
+    return array(
+      'name' => 'User role administration',
+      'description' => 'Test adding, editing and deleting user roles.',
+      'group' => 'User',
+    );
+  }
+
+  function setUp() {
+    parent::setUp();
+    $this->admin_user = $this->drupalCreateUser(array('administer permissions', 'administer users'));
+  }
+
+  /**
+   * Test adding, renaming and deleting roles.
+   */
+  function testRoleAdministration() {
+    $this->drupalLogin($this->admin_user);
+    // Test adding a role.
+    $role_name = $this->randomName();
+    $edit = array('name' => $role_name);
+    $this->drupalPost('admin/people/permissions/roles', $edit, t('Add role'));
+    $this->assertText(t('The role has been added.'), t('The role has been added.'));
+    $role = user_role_load_by_name($role_name);
+    $this->assertTrue(is_object($role), t('The role was successfully retrieved from the database.'));
+
+    // Try adding a duplicate role.
+    $this->drupalPost(NULL, $edit, t('Add role'));
+    $this->assertRaw(t('The role name %name already exists. Choose another role name.', array('%name' => $role_name)), t('Duplicate role warning displayed.'));
+
+    // Renaming a role
+    $old_name = $role_name;
+    $role_name = $this->randomName();
+    $edit = array('name' => $role_name);
+    $this->drupalPost("admin/people/permissions/roles/edit/{$role->rid}", $edit, t('Save role'));
+    $this->assertText(t('The role has been renamed.'), t('The role has been renamed.'));
+    $this->assertFalse(user_role_load_by_name($old_name), t('The role can no longer be retrieved from the database using its old name.'));
+    $this->assertTrue(is_object(user_role_load_by_name($role_name)), t('The role can be loaded from the database using its new name.'));
+
+    // Deleting a role.
+    $this->drupalPost("admin/people/permissions/roles/edit/{$role->rid}", NULL, t('Delete role'));
+    $this->assertText(t('The role has been deleted.'), t('The role has been deleted'));
+    $this->assertNoLinkByHref("admin/people/permissions/roles/edit/{$role->rid}", t('Role edit link removed.'));
+    $this->assertFalse(user_role_load_by_name($role_name), t('Deleted role can no longer be loaded.'));
+  }
 }
\ No newline at end of file
Index: modules/user/user.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/user/user.module,v
retrieving revision 1.1137
diff -u -r1.1137 user.module
--- modules/user/user.module	18 Mar 2010 06:43:41 -0000	1.1137
+++ modules/user/user.module	18 Mar 2010 16:43:58 -0000
@@ -1560,10 +1560,11 @@
     'type' => MENU_LOCAL_TASK,
     'weight' => -5,
   );
-  $items['admin/people/permissions/roles/edit'] = array(
+  $items['admin/people/permissions/roles/edit/%user_role'] = array(
     'title' => 'Edit role',
-    'page arguments' => array('user_admin_role'),
-    'access arguments' => array('administer permissions'),
+    'page arguments' => array('user_admin_role', 5),
+    'access callback' => 'user_role_edit_access',
+    'access arguments' => array(5),
     'type' => MENU_CALLBACK,
   );
 
@@ -2563,22 +2564,43 @@
 }
 
 /**
- * Fetch a user role from database.
+ * Fetches a user role by role ID.
  *
  * @param $role
- *   A string with the role name, or an integer with the role ID.
+ *   An integer with the role ID.
  * @return
  *   A fully-loaded role object if a role with the given name or ID
  *   exists, FALSE otherwise.
+ *
+ * @see user_role_load_by_name
  */
-function user_role_load($role) {
-  $field = is_int($role) ? 'rid' : 'name';
+function user_role_load($rid) {
   return db_select('role', 'r')
     ->fields('r')
-    ->condition($field, $role)
+    ->condition('rid', $rid)
     ->execute()
     ->fetchObject();
 }
+
+/**
+ * Fetches a user role by role name.
+ *
+ * @param $role
+ *   A string with the role name.
+ * @return
+ *   A fully-loaded role object if a role with the given name exists, FALSE
+ *   otherwise.
+ *
+ * @see user_role_load
+ */
+function user_role_load_by_name($role) {
+  return db_select('role', 'r')
+    ->fields('r')
+    ->condition('name', $role)
+    ->execute()
+    ->fetchObject();
+}
+
 /**
  * Save a user role to the database.
  *
@@ -2619,7 +2641,12 @@
  *   A string with the role name, or an integer with the role ID.
  */
 function user_role_delete($role) {
-  $role = user_role_load($role);
+  if (is_int($role)) {
+    $role = user_role_load($role);
+  }
+  else {
+    $role = user_role_load_by_name($role);
+  }
 
   db_delete('role')
     ->condition('rid', $role->rid)
@@ -2640,6 +2667,19 @@
 }
 
 /**
+ * Menu access callback for user role editing.
+ */
+function user_role_edit_access($role) {
+  // Prevent the system-defined roles (Anonymous user and Authenticated user)
+  // from being altered or removed.
+  if (isset($role) && ($role->rid == 1 || $role->rid == 2)) {
+    return FALSE;
+  }
+
+  return user_access('administer permissions');
+}
+
+/**
  * Determine the modules that permissions belong to.
  *
  * @return
