diff --git a/ChangeLog b/ChangeLog
index fd0f3ae93..9a3d11df5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2006-02-27  Elijah Newren  <newren gmail com>
+
+        Patch from Thomas Thurman to prevent setting cycle_windows to
+        keybindings that won't work.  #329676
+
+        * src/prefs.c (update_binding):
+        Make sure that bindings which require a modifier key are set to a
+        keybinding with one or else that the binding is reverted.
+
+        * src/ui.[ch] (meta_ui_accelerator_name):
+        New function
+
 2006-02-27  Elijah Newren  <newren gmail com>
 
         Patch from Thomas Thurman to work around buggy application
diff --git a/src/prefs.c b/src/prefs.c
index 5683ac400..8e762fb6e 100644
--- a/src/prefs.c
+++ b/src/prefs.c
@@ -2030,8 +2030,70 @@ update_binding (MetaKeyPref *binding,
                       "Failed to parse new gconf value\n");
           meta_warning (_("\"%s\" found in configuration database is not a valid value for keybinding \"%s\"\n"),
                         value, binding->name);
+
+          return FALSE;
         }
     }
+
+  /* Bug 329676: Bindings which can be shifted must not have no modifiers,
+   * nor only SHIFT as a modifier.
+   */
+
+  if (binding->add_shift &&
+      0 != keysym &&
+      (META_VIRTUAL_SHIFT_MASK == mods || 0 == mods))
+    {
+      gchar *old_setting;
+      gchar *key;
+      GError *err = NULL;
+      
+      meta_warning ("Cannot bind \"%s\" to %s: it needs a modifier "
+                    "such as Ctrl or Alt.\n",
+                    binding->name,
+                    value);
+
+      old_setting = meta_ui_accelerator_name(
+                      binding->keysym,
+                      binding->modifiers);
+      
+      if (!strcmp(old_setting, value))
+        {
+          /* We were about to set it to the same value
+           * that it had originally! This must be caused
+           * by getting an invalid string back from
+           * meta_ui_accelerator_name. Bail out now
+           * so we don't get into an infinite loop.
+           */
+           return TRUE;
+        }
+
+      meta_warning ("Reverting \"%s\" to %s.\n",
+                    binding->name,
+                    old_setting);
+
+      key = g_strconcat (KEY_SCREEN_BINDINGS_PREFIX, "/",
+                         binding->name, NULL);
+      
+      gconf_client_set_string (gconf_client_get_default (),
+                               key, old_setting, &err);
+
+      if (err)
+        {
+          meta_warning ("Error while reverting keybinding: %s\n",
+                        err->message);
+          g_error_free (err);
+          err = NULL;
+        }
+      
+      g_free (old_setting);
+      g_free (key);
+
+      /* The call to gconf_client_set_string() will cause this function
+       * to be called again with the new value, so there's no need to
+       * carry on.
+       */
+      return TRUE;
+    }
   
   changed = FALSE;
   if (keysym != binding->keysym ||
diff --git a/src/ui.c b/src/ui.c
index 3640cb0ff..3f10044cc 100644
--- a/src/ui.c
+++ b/src/ui.c
@@ -788,6 +788,43 @@ meta_ui_parse_accelerator (const char          *accel,
   return TRUE;
 }
 
+/* Caller responsible for freeing return string of meta_ui_accelerator_name! */
+gchar*
+meta_ui_accelerator_name  (unsigned int        keysym,
+                           MetaVirtualModifier mask)
+{
+  EggVirtualModifierType mods = 0;
+        
+  if (keysym == 0 && mask == 0)
+    {
+      return strdup ("disabled");
+    }
+
+  if (mask & META_VIRTUAL_SHIFT_MASK)
+    mods |= EGG_VIRTUAL_SHIFT_MASK;
+  if (mask & META_VIRTUAL_CONTROL_MASK)
+    mods |= EGG_VIRTUAL_CONTROL_MASK;
+  if (mask & META_VIRTUAL_ALT_MASK)
+    mods |= EGG_VIRTUAL_ALT_MASK;
+  if (mask & META_VIRTUAL_MOD2_MASK)
+    mods |= EGG_VIRTUAL_MOD2_MASK;
+  if (mask & META_VIRTUAL_MOD3_MASK)
+    mods |= EGG_VIRTUAL_MOD3_MASK;
+  if (mask & META_VIRTUAL_MOD4_MASK)
+    mods |= EGG_VIRTUAL_MOD4_MASK;
+  if (mask & META_VIRTUAL_MOD5_MASK)
+    mods |= EGG_VIRTUAL_MOD5_MASK;
+  if (mask & META_VIRTUAL_SUPER_MASK)
+    mods |= EGG_VIRTUAL_SUPER_MASK;
+  if (mask & META_VIRTUAL_HYPER_MASK)
+    mods |= EGG_VIRTUAL_HYPER_MASK;
+  if (mask & META_VIRTUAL_META_MASK)
+    mods |= EGG_VIRTUAL_META_MASK;
+
+  return egg_virtual_accelerator_name (keysym, mods);
+
+}
+
 gboolean
 meta_ui_parse_modifier (const char          *accel,
                         MetaVirtualModifier *mask)
diff --git a/src/ui.h b/src/ui.h
index 2d4af4a09..e2a1833a0 100644
--- a/src/ui.h
+++ b/src/ui.h
@@ -185,6 +185,9 @@ gboolean meta_ui_parse_accelerator (const char          *accel,
 gboolean meta_ui_parse_modifier    (const char          *accel,
                                     MetaVirtualModifier *mask);
 
+/* Caller responsible for freeing return string of meta_ui_accelerator_name! */
+gchar*   meta_ui_accelerator_name  (unsigned int        keysym,
+                                    MetaVirtualModifier mask);
 gboolean meta_ui_window_is_widget (MetaUI *ui,
                                    Window  xwindow);