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

3 files changed, 91 insertions, 0 deletions

diff --git a/modules/shortcut/shortcut.admin.inc b/modules/shortcut/shortcut.admin.inc
index e86d8f50f..27384a669 100644
--- a/modules/shortcut/shortcut.admin.inc
+++ b/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) {
@@ -102,6 +103,22 @@ function shortcut_set_switch($form, &$form_state, $account = NULL) {
 }
 
 /**
+ * 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().
  */
 function shortcut_set_switch_submit($form, &$form_state) {
@@ -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');
@@ -203,6 +222,16 @@ function shortcut_set_add_form($form, &$form_state) {
 }
 
 /**
+ * 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().
  */
 function shortcut_set_add_form_submit($form, &$form_state) {
@@ -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) {
@@ -573,6 +603,17 @@ function shortcut_set_edit_form($form, &$form_state, $shortcut_set) {
 }
 
 /**
+ * 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().
  */
 function shortcut_set_edit_form_submit($form, &$form_state) {
diff --git a/modules/shortcut/shortcut.module b/modules/shortcut/shortcut.module
index 7144dae93..8e9773de1 100644
--- a/modules/shortcut/shortcut.module
+++ b/modules/shortcut/shortcut.module
@@ -583,6 +583,19 @@ function shortcut_sets() {
 }
 
 /**
+ * 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.
  *
  * @param $path
diff --git a/modules/shortcut/shortcut.test b/modules/shortcut/shortcut.test
index fba8cb7c1..34ae06183 100644
--- a/modules/shortcut/shortcut.test
+++ b/modules/shortcut/shortcut.test
@@ -239,6 +239,31 @@ class ShortcutSetsTestCase extends ShortcutTestCase {
   }
 
   /**
+   * 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.
    */
   function testShortcutSetSave() {
@@ -266,6 +291,18 @@ class ShortcutSetsTestCase extends ShortcutTestCase {
   }
 
   /**
+   * 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.
    */
   function testShortcutSetDelete() {