- Patch #800366 by sivaji, David_Rothstein: prevent creating shortcut set with empty or duplicate label.

modules/shortcut/shortcut.admin.inc

@@ -33,6 +33,7 @@ function shortcut_max_slots() {
  *   An array representing the form definition.
  *
  * @ingroup forms
+ * @see shortcut_set_switch_validate()
  * @see shortcut_set_switch_submit()
  */
 function shortcut_set_switch($form, &$form_state, $account = NULL) {
@@ -101,6 +102,22 @@ function shortcut_set_switch($form, &$form_state, $account = NULL) {
   return $form;
 }
 
+/**
+ * Validation handler for shortcut_set_switch().
+ */
+function shortcut_set_switch_validate($form, &$form_state) {
+  if ($form_state['values']['set'] == 'new') {
+    // Check to prevent creating a shortcut set with an empty title.
+    if (trim($form_state['values']['new']) == '') {
+      form_set_error('new', t('The new set name is required.'));
+    }
+    // Check to prevent a duplicate title.
+    if (shortcut_set_title_exists($form_state['values']['new'])) {
+      form_set_error('new', t('The shortcut set %name already exists. Choose another name.', array('%name' => $form_state['values']['new'])));
+    }
+  }
+}
+
 /**
  * Submit handler for shortcut_set_switch().
  */
@@ -184,6 +201,7 @@ function shortcut_set_admin() {
  *   An array representing the form definition.
  *
  * @ingroup forms
+ * @see shortcut_set_add_form_validate()
  * @see shortcut_set_add_form_submit()
  */
 function shortcut_set_add_form($form, &$form_state) {
@@ -191,6 +209,7 @@ function shortcut_set_add_form($form, &$form_state) {
     '#type' => 'textfield',
     '#title' => t('Set name'),
     '#description' => t('The new set is created by copying items from your default shortcut set.'),
+    '#required' => TRUE,
   );
 
   $form['actions'] = array('#type' => 'actions');
@@ -202,6 +221,16 @@ function shortcut_set_add_form($form, &$form_state) {
   return $form;
 }
 
+/**
+ * Validation handler for shortcut_set_add_form().
+ */
+function shortcut_set_add_form_validate($form, &$form_state) {
+  // Check to prevent a duplicate title.
+  if (shortcut_set_title_exists($form_state['values']['new'])) {
+    form_set_error('new', t('The shortcut set %name already exists. Choose another name.', array('%name' => $form_state['values']['new'])));
+  }
+}
+
 /**
  * Submit handler for shortcut_set_add_form().
  */
@@ -547,6 +576,7 @@ function shortcut_admin_add_link($shortcut_link, &$shortcut_set, $limit = NULL)
  *   An array representing the form definition.
  *
  * @ingroup forms
+ * @see shortcut_set_edit_form_validate()
  * @see shortcut_set_edit_form_submit()
  */
 function shortcut_set_edit_form($form, &$form_state, $shortcut_set) {
@@ -572,6 +602,17 @@ function shortcut_set_edit_form($form, &$form_state, $shortcut_set) {
   return $form;
 }
 
+/**
+ * Validation handler for shortcut_set_edit_form().
+ */
+function shortcut_set_edit_form_validate($form, &$form_state) {
+  // Check to prevent a duplicate title, if the title was edited from its
+  // original value.
+  if ($form_state['values']['title'] != $form_state['values']['shortcut_set']->title && shortcut_set_title_exists($form_state['values']['title'])) {
+    form_set_error('title', t('The shortcut set %name already exists. Choose another name.', array('%name' => $form_state['values']['title'])));
+  }
+}
+
 /**
  * Submit handler for shortcut_set_edit_form().
  */

modules/shortcut/shortcut.module

@@ -582,6 +582,19 @@ function shortcut_sets() {
   ->fetchAllAssoc('set_name');
 }
 
+/**
+ * Check to see if a shortcut set with the given title already exists.
+ *
+ * @param $title
+ *   Human-readable name of the shortcut set to check.
+ *
+ * @return
+ *   TRUE if a shortcut set with that title exists; FALSE otherwise.
+ */
+function shortcut_set_title_exists($title) {
+  return (bool) db_query_range('SELECT 1 FROM {shortcut_set} WHERE title = :title', 0, 1, array(':title' => $title))->fetchField();
+}
+
 /**
  * Determines if a path corresponds to a valid shortcut link.
  *

modules/shortcut/shortcut.test

@@ -238,6 +238,31 @@ class ShortcutSetsTestCase extends ShortcutTestCase {
     $this->assertTrue($new_set->set_name == $current_set->set_name, "Successfully switched another user's shortcut set.");
   }
 
+  /**
+   * Tests switching a user's shortcut set and creating one at the same time.
+   */
+  function testShortcutSetSwitchCreate() {
+    $edit = array(
+      'set' => 'new',
+      'new' => $this->randomName(10),
+    );
+    $this->drupalPost('user/' . $this->admin_user->uid . '/shortcuts', $edit, t('Change set'));
+    $current_set = shortcut_current_displayed_set($this->admin_user);
+    $this->assertNotEqual($current_set->set_name, $this->set->set_name, 'A shortcut set can be switched to at the same time as it is created.');
+    $this->assertEqual($current_set->title, $edit['new'], 'The new set is correctly assigned to the user.');
+  }
+
+  /**
+   * Tests switching a user's shortcut set without providing a new set name.
+   */
+  function testShortcutSetSwitchNoSetName() {
+    $edit = array('set' => 'new');
+    $this->drupalPost('user/' . $this->admin_user->uid . '/shortcuts', $edit, t('Change set'));
+    $this->assertText(t('The new set name is required.'));
+    $current_set = shortcut_current_displayed_set($this->admin_user);
+    $this->assertEqual($current_set->set_name, $this->set->set_name, 'Attempting to switch to a new shortcut set without providing a set name does not succeed.');
+  }
+
   /**
    * Tests that shortcut_set_save() correctly updates existing links.
    */
@@ -265,6 +290,18 @@ class ShortcutSetsTestCase extends ShortcutTestCase {
     $this->assertTrue($set->title == $new_title, 'Shortcut set has been successfully renamed.');
   }
 
+  /**
+   * Tests renaming a shortcut set to the same name as another set.
+   */
+  function testShortcutSetRenameAlreadyExists() {
+    $set = $this->generateShortcutSet($this->randomName(10));
+    $existing_title = $this->set->title;
+    $this->drupalPost('admin/config/user-interface/shortcut/' . $set->set_name . '/edit', array('title' => $existing_title), t('Save'));
+    $this->assertRaw(t('The shortcut set %name already exists. Choose another name.', array('%name' => $existing_title)));
+    $set = shortcut_set_load($set->set_name);
+    $this->assertNotEqual($set->title, $existing_title, t('The shortcut set %title cannot be renamed to %new-title because a shortcut set with that title already exists.', array('%title' => $set->title, '%new-title' => $existing_title)));
+  }
+
   /**
    * Tests deleting a shortcut set.
    */