Assorted UI fixes:

- Change sort functions to be on entities, so can use on the view pages too
- Fix bug: empty ajax search on useredit blows up
- Filter illegal characters from suggest uid/email methods
- Rename first/last name fields
- Make default font family sans-serif
- Speed up effect appear/fade rendering
- Add buttons to top and bottom of pages
- Make grouplist sortable
- Add noscript warning to welcome page

14 files changed, 122 insertions, 44 deletions

diff --git a/ipa-server/ipa-gui/ipagui/controllers.py b/ipa-server/ipa-gui/ipagui/controllers.py
index 9e8f7114f..41bd3b293 100644
--- a/ipa-server/ipa-gui/ipagui/controllers.py
+++ b/ipa-server/ipa-gui/ipagui/controllers.py
@@ -1,6 +1,7 @@
 import random
 from pickle import dumps, loads
 from base64 import b64encode, b64decode
+import re
 
 import os
 import cherrypy
@@ -50,40 +51,40 @@ def utf8_encode(value):
 
 def sort_group_member(a, b):
     """Comparator function used for sorting group members."""
-    if a.get('uid') and b.get('uid'):
-        if a.get('givenname', '') == b.get('givenname', ''):
-            if a.get('sn', '') == b.get('sn', ''):
-                if a.get('uid') == b.get('uid'):
+    if a.getValue('uid') and b.getValue('uid'):
+        if a.getValue('givenname') == b.getValue('givenname'):
+            if a.getValue('sn') == b.getValue('sn'):
+                if a.getValue('uid') == b.getValue('uid'):
                     return 0
-                elif a.get('uid') < b.get('uid'):
+                elif a.getValue('uid') < b.getValue('uid'):
                     return -1
                 else:
                     return 1
-            elif a.get('sn', '') < b.get('sn', ''):
+            elif a.getValue('sn') < b.getValue('sn'):
                 return -1
             else:
                 return 1
-        elif a.get('givenname') < b.get('givenname'):
+        elif a.getValue('givenname') < b.getValue('givenname'):
             return -1
         else:
             return 1
-    elif a.get('uid'):
+    elif a.getValue('uid'):
         return -1
-    elif b.get('uid'):
+    elif b.getValue('uid'):
         return 1
     else:
-        if a.get('cn', '') == b.get('cn', ''):
+        if a.getValue('cn') == b.getValue('cn'):
             return 0
-        elif a.get('cn', '') < b.get('cn', ''):
+        elif a.getValue('cn') < b.getValue('cn'):
             return -1
         else:
             return 1
 
 def sort_by_cn(a, b):
     """Comparator function used for sorting groups."""
-    if a.get('cn', '') == b.get('cn', ''):
+    if a.getValue('cn') == b.getValue('cn'):
         return 0
-    elif a.get('cn', '') < b.get('cn', ''):
+    elif a.getValue('cn') < b.getValue('cn'):
         return -1
     else:
         return 1
@@ -216,7 +217,7 @@ class Root(controllers.RootController):
            This method is used for the ajax search on the user edit page."""
         client.set_krbccache(os.environ["KRB5CCNAME"])
         groups = []
-        counter = 0
+        groups_counter = 0
         searchlimit = 100
         criteria = kw.get('criteria')
         if criteria != None and len(criteria) > 0:
@@ -248,8 +249,8 @@ class Root(controllers.RootController):
                 del(user_dict['userpassword'])
 
             user_groups = client.get_groups_by_member(user.dn, ['dn', 'cn'])
+            user_groups.sort(sort_by_cn)
             user_groups_dicts = map(lambda group: group.toDict(), user_groups)
-            user_groups_dicts.sort(sort_by_cn)
             user_groups_data = b64encode(dumps(user_groups_dicts))
 
             # store a copy of the original user for the update later
@@ -421,8 +422,10 @@ class Root(controllers.RootController):
         try:
             user = client.get_user_by_uid(uid, user_fields)
             user_groups = client.get_groups_by_member(user.dn, ['cn'])
+            user_groups.sort(sort_by_cn)
             user_reports = client.get_users_by_manager(user.dn,
                     ['givenname', 'sn', 'uid'])
+            user_reports.sort(sort_group_member)
 
             user_manager = None
             try:
@@ -466,6 +469,10 @@ class Root(controllers.RootController):
     @expose()
     @identity.require(identity.not_anonymous())
     def suggest_uid(self, givenname, sn):
+        # filter illegal uid characters out
+        givenname = re.sub(r'[^a-zA-Z_\-0-9]', "", givenname)
+        sn = re.sub(r'[^a-zA-Z_\-0-9]', "", sn)
+
         if (len(givenname) == 0) or (len(sn) == 0):
             return ""
 
@@ -512,6 +519,10 @@ class Root(controllers.RootController):
     @expose()
     @identity.require(identity.not_anonymous())
     def suggest_email(self, givenname, sn):
+        # remove illegal email characters
+        givenname = re.sub(r'[^a-zA-Z0-9!#\$%\*/?\|\^\{\}`~&\'\+\-=_]', "", givenname)
+        sn = re.sub(r'[^a-zA-Z0-9!#\$%\*/?\|\^\{\}`~&\'\+\-=_]', "", sn)
+
         if (len(givenname) == 0) or (len(sn) == 0):
             return ""
 
@@ -716,11 +727,11 @@ class Root(controllers.RootController):
                     lambda dn: client.get_user_by_dn(dn, ['dn', 'givenname', 'sn',
                         'uid', 'cn']),
                     member_dns)
+            members.sort(sort_group_member)
 
             # Map users into an array of dicts, which can be serialized
             # (so we don't have to do this on each round trip)
             member_dicts = map(lambda member: member.toDict(), members)
-            member_dicts.sort(sort_group_member)
 
             # store a copy of the original group for the update later
             group_data = b64encode(dumps(group_dict))
@@ -885,8 +896,8 @@ class Root(controllers.RootController):
                     lambda dn: client.get_user_by_dn(dn, ['dn', 'givenname', 'sn',
                         'uid', 'cn']),
                     member_dns)
+            members.sort(sort_group_member)
             member_dicts = map(lambda member: member.toDict(), members)
-            member_dicts.sort(sort_group_member)
 
             return dict(group=group_dict, fields=forms.group.GroupFields(),
                     members = member_dicts)
diff --git a/ipa-server/ipa-gui/ipagui/forms/user.py b/ipa-server/ipa-gui/ipagui/forms/user.py
index 6211c9ff3..8beac5100 100644
--- a/ipa-server/ipa-gui/ipagui/forms/user.py
+++ b/ipa-server/ipa-gui/ipagui/forms/user.py
@@ -8,9 +8,9 @@ class UserFields():
             label="Confirm Password")
     uidnumber = widgets.TextField(name="uidnumber", label="UID")
     gidnumber = widgets.TextField(name="gidnumber", label="GID")
-    givenname = widgets.TextField(name="givenname", label="First name")
-    sn = widgets.TextField(name="sn", label="Last name")
-    mail = widgets.TextField(name="mail", label="E-mail address")
+    givenname = widgets.TextField(name="givenname", label="Given Name")
+    sn = widgets.TextField(name="sn", label="Family Name")
+    mail = widgets.TextField(name="mail", label="E-mail Address")
     telephonenumber = widgets.TextField(name="telephonenumber", label="Phone")
     # nsAccountLock = widgets.CheckBox(name="nsAccountLock", label="Account Deactivated")
     nsAccountLock = widgets.SingleSelectField(name="nsAccountLock",
diff --git a/ipa-server/ipa-gui/ipagui/static/css/style.css b/ipa-server/ipa-gui/ipagui/static/css/style.css
index b3ed830d1..ae845e863 100644
--- a/ipa-server/ipa-gui/ipagui/static/css/style.css
+++ b/ipa-server/ipa-gui/ipagui/static/css/style.css
@@ -7,6 +7,7 @@ html, body {
   background:#fff;
   margin: 0;
   padding: 0;
+  font-family: sans-serif;
 }
 
 body {
diff --git a/ipa-server/ipa-gui/ipagui/static/javascript/dynamicedit.js b/ipa-server/ipa-gui/ipagui/static/javascript/dynamicedit.js
index 05c22998a..b7dd95ea1 100644
--- a/ipa-server/ipa-gui/ipagui/static/javascript/dynamicedit.js
+++ b/ipa-server/ipa-gui/ipagui/static/javascript/dynamicedit.js
@@ -102,7 +102,7 @@ function addmember(dn, info) {
   var undolink = document.createElement('a');
   undolink.setAttribute('href', '');
   undolink.setAttribute('onclick',
-    'new Effect.Fade(Element.up(this), {afterFinish: removeElement});' +
+    'new Effect.Fade(Element.up(this), {afterFinish: removeElement, duration: 0.75});' +
     'added_hash.remove("' + jsStringEscape(dn) + '");' +
     'return false;');
   undolink.appendChild(document.createTextNode("undo"));
@@ -123,8 +123,8 @@ function addmember(dn, info) {
 function addmemberHandler(element, dn, info) {
   var newdiv = addmember(dn, info)
   if (newdiv != null) {
-    new Effect.Fade(Element.up(element));
-    new Effect.Appear(newdiv);
+    new Effect.Fade(Element.up(element), {duration: 0.75});
+    new Effect.Appear(newdiv, {duration: 0.75});
     /* Element.up(element).remove(); */
   }
 }
@@ -139,8 +139,8 @@ function removemember(dn, info) {
   var undolink = document.createElement('a');
   undolink.setAttribute('href', '');
   undolink.setAttribute('onclick',
-    'new Effect.Fade(Element.up(this), {afterFinish: removeElement});' +
-    "new Effect.Appear($('" + orig_div_id + "'));" +
+    'new Effect.Fade(Element.up(this), {afterFinish: removeElement, duration: 0.75});' +
+    "new Effect.Appear($('" + orig_div_id + "'), {duration: 0.75});" +
     'return false;');
   undolink.appendChild(document.createTextNode("undo"));
   newdiv.appendChild(undolink);
@@ -159,8 +159,8 @@ function removemember(dn, info) {
 
 function removememberHandler(element, dn, info) {
   var newdiv = removemember(dn, info);
-  new Effect.Fade(Element.up(element));
-  new Effect.Appear(newdiv);
+  new Effect.Fade(Element.up(element), {duration: 0.75});
+  new Effect.Appear(newdiv, {duration: 0.75});
   /* Element.up(element).remove(); */
 }
 
diff --git a/ipa-server/ipa-gui/ipagui/static/javascript/tablekit.js b/ipa-server/ipa-gui/ipagui/static/javascript/tablekit.js
index 2851f8431..540369489 100644
--- a/ipa-server/ipa-gui/ipagui/static/javascript/tablekit.js
+++ b/ipa-server/ipa-gui/ipagui/static/javascript/tablekit.js
@@ -291,7 +291,9 @@ TableKit.Sortable = {
 		
 		if(cell.hasClassName(op.noSortClass)) {return;}	
 		
-		order = order ? order : (cell.hasClassName(op.descendingClass) ? 1 : -1);
+		// order = order ? order : (cell.hasClassName(op.descendingClass) ? 1 : -1);
+                // kmccarth - change default sort order to ascending
+		order = order ? order : (cell.hasClassName(op.ascendingClass) ? -1 : 1);
 		var rows = TableKit.getBodyRows(table);
 
 		if(cell.hasClassName(op.ascendingClass) || cell.hasClassName(op.descendingClass)) {
@@ -843,4 +845,4 @@ if(window.FastInit) {
 	FastInit.addOnLoad(TableKit.load);
 } else {
 	Event.observe(window, 'load', TableKit.load);
-}
\ No newline at end of file
+}
diff --git a/ipa-server/ipa-gui/ipagui/templates/dynamiceditsearch.kid b/ipa-server/ipa-gui/ipagui/templates/dynamiceditsearch.kid
index f69b11213..f0414216d 100644
--- a/ipa-server/ipa-gui/ipagui/templates/dynamiceditsearch.kid
+++ b/ipa-server/ipa-gui/ipagui/templates/dynamiceditsearch.kid
@@ -66,7 +66,7 @@ from ipagui.helpers import ipahelper
     </div>
     <script type="text/javascript">
       if (results_counter == 0) {
-        var message = "No results found for " + search_string;
+        var message = "No results found for '" + search_string + "'";
       } else {
         var message =  results_counter + " results found:";
       }
diff --git a/ipa-server/ipa-gui/ipagui/templates/groupeditform.kid b/ipa-server/ipa-gui/ipagui/templates/groupeditform.kid
index df1f0ccbb..55ccfdf09 100644
--- a/ipa-server/ipa-gui/ipagui/templates/groupeditform.kid
+++ b/ipa-server/ipa-gui/ipagui/templates/groupeditform.kid
@@ -3,6 +3,19 @@
   <form action="${action}" name="${name}" method="${method}" class="tableform"
       onsubmit="preSubmit()" >
 
+    <table class="formtable" cellpadding="2" cellspacing="0" border="0">
+      <tr>
+        <th>
+          <input type="submit" class="submitbutton" name="submit"
+              value="Update Group"/>
+        </th>
+        <td>
+          <input type="submit" class="submitbutton" name="submit"
+              value="Cancel Edit" />
+        </td>
+      </tr>
+    </table>
+
 <?python
 from ipagui.helpers import ipahelper
 ?>
@@ -163,8 +176,6 @@ from ipagui.helpers import ipahelper
       </div>
     </div>
 
-
-
     <table class="formtable" cellpadding="2" cellspacing="0" border="0">
       <tr>
         <th>
diff --git a/ipa-server/ipa-gui/ipagui/templates/grouplist.kid b/ipa-server/ipa-gui/ipagui/templates/grouplist.kid
index c867ecba0..2fbefa4b7 100644
--- a/ipa-server/ipa-gui/ipagui/templates/grouplist.kid
+++ b/ipa-server/ipa-gui/ipagui/templates/grouplist.kid
@@ -6,6 +6,7 @@
 <title>Find Groups</title>
 </head>
 <body>
+    <script type="text/javascript" charset="utf-8" src="${tg.url('/static/javascript/tablekit.js')}"></script>
     <div id="search">
         <form action="${tg.url('/grouplist')}" method="get">
             <input id="criteria" type="text" name="criteria" value="${criteria}" />
@@ -17,15 +18,18 @@
     </div>
     <div py:if='(groups != None) and (len(groups) > 0)'>
         <h2>${len(groups)} results returned:</h2>
-        <table id="resultstable">
+        <table id="resultstable" class="sortable resizable">
+          <thead>
             <tr>
                 <th>
-                    <label class="fieldlabel" py:content="fields.cn.label" />
+                    ${fields.cn.label}
                 </th>
                 <th>
-                    <label class="fieldlabel" py:content="fields.description.label" />
+                    ${fields.description.label}
                 </th>
             </tr>
+          </thead>
+          <tbody>
             <tr py:for="group in groups">
                 <td>
                     <a href="${tg.url('/groupshow',cn=group.cn)}">${group.cn}</a>
@@ -34,6 +38,7 @@
                     ${group.description}
                 </td>
             </tr>
+          </tbody>
         </table>
     </div>
     <div py:if='(groups != None) and (len(groups) == 0)'>
diff --git a/ipa-server/ipa-gui/ipagui/templates/groupnewform.kid b/ipa-server/ipa-gui/ipagui/templates/groupnewform.kid
index 17fd9013a..29c79cfb9 100644
--- a/ipa-server/ipa-gui/ipagui/templates/groupnewform.kid
+++ b/ipa-server/ipa-gui/ipagui/templates/groupnewform.kid
@@ -3,6 +3,14 @@
   <form action="${action}" name="${name}" method="${method}" class="tableform"
       onsubmit="preSubmit()" >
 
+    <table class="formtable" cellpadding="2" cellspacing="0" border="0">
+      <tr>
+        <td>
+          <input type="submit" class="submitbutton" name="submit" value="Add Group"/>
+        </td>
+      </tr>
+    </table>
+
 <?python
 from ipagui.helpers import ipahelper
 ?>
@@ -91,7 +99,6 @@ from ipagui.helpers import ipahelper
 
     <table class="formtable" cellpadding="2" cellspacing="0" border="0">
       <tr>
-        <th></th>
         <td>
           <br />
           <input type="submit" class="submitbutton" name="submit" value="Add Group"/>
diff --git a/ipa-server/ipa-gui/ipagui/templates/groupshow.kid b/ipa-server/ipa-gui/ipagui/templates/groupshow.kid
index a79f268cc..3bbf3c336 100644
--- a/ipa-server/ipa-gui/ipagui/templates/groupshow.kid
+++ b/ipa-server/ipa-gui/ipagui/templates/groupshow.kid
@@ -6,8 +6,15 @@
     <title>View Group</title>
 </head>
 <body>
+<?python
+edit_url = tg.url('/groupedit', cn=group.get('cn'))
+?>
     <h2>View Group</h2>
 
+    <input type="button"
+      onclick="document.location.href='${edit_url}'"
+      value="Edit Group" />
+
     <div class="formsection">Group Details</div>
     <table class="formtable" cellpadding="2" cellspacing="0" border="0">
         <tr>
@@ -61,9 +68,10 @@
     </div>
 
     <br/>
-    <br/>
 
-    <a href="${tg.url('/groupedit', cn=group.get('cn'))}">edit</a>
+    <input type="button"
+      onclick="document.location.href='${edit_url}'"
+      value="Edit Group" />
 
 </body>
 </html>
diff --git a/ipa-server/ipa-gui/ipagui/templates/usereditform.kid b/ipa-server/ipa-gui/ipagui/templates/usereditform.kid
index 64d28f634..135f352a0 100644
--- a/ipa-server/ipa-gui/ipagui/templates/usereditform.kid
+++ b/ipa-server/ipa-gui/ipagui/templates/usereditform.kid
@@ -3,6 +3,20 @@
   <form action="${action}" name="${name}" method="${method}" class="tableform"
     onsubmit="preSubmit()">
 
+    <table class="formtable" cellpadding="2" cellspacing="0" border="0">
+      <tr>
+        <th>
+          <input type="submit" class="submitbutton" name="submit"
+              value="Update Person"/>
+        </th>
+        <td>
+          <input type="submit" class="submitbutton" name="submit"
+              value="Cancel Edit" />
+        </td>
+        <td></td>
+      </tr>
+    </table>
+
 <?python
 from ipagui.helpers import ipahelper
 ?>
@@ -314,9 +328,6 @@ from ipagui.helpers import ipahelper
       </div>
     </div>
 
-
-
-
     <table class="formtable" cellpadding="2" cellspacing="0" border="0">
       <tr>
         <th>
diff --git a/ipa-server/ipa-gui/ipagui/templates/usernewform.kid b/ipa-server/ipa-gui/ipagui/templates/usernewform.kid
index a8bd991d2..fd79e5ed8 100644
--- a/ipa-server/ipa-gui/ipagui/templates/usernewform.kid
+++ b/ipa-server/ipa-gui/ipagui/templates/usernewform.kid
@@ -3,6 +3,14 @@
   <form action="${action}" name="${name}" method="${method}" class="tableform"
     onsubmit="preSubmit()">
 
+    <table class="formtable" cellpadding="2" cellspacing="0" border="0">
+      <tr>
+        <td>
+          <input type="submit" class="submitbutton" name="submit" value="Add Person"/>
+        </td>
+      </tr>
+    </table>
+
 <?python
 from ipagui.helpers import ipahelper
 ?>
@@ -240,7 +248,6 @@ from ipagui.helpers import ipahelper
 
     <table class="formtable" cellpadding="2" cellspacing="0" border="0">
       <tr>
-        <th></th>
         <td>
           <br />
           <input type="submit" class="submitbutton" name="submit" value="Add Person"/>
diff --git a/ipa-server/ipa-gui/ipagui/templates/usershow.kid b/ipa-server/ipa-gui/ipagui/templates/usershow.kid
index ce83f5942..818dd78b5 100644
--- a/ipa-server/ipa-gui/ipagui/templates/usershow.kid
+++ b/ipa-server/ipa-gui/ipagui/templates/usershow.kid
@@ -6,8 +6,15 @@
     <title>View Person</title>
 </head>
 <body>
+<?python
+edit_url = tg.url('/useredit', uid=user.get('uid'))
+?>
     <h2>View Person</h2>
 
+    <input type="button"
+      onclick="document.location.href='${edit_url}'"
+      value="Edit Person" />
+
 <?python
 from ipagui.helpers import userhelper
 pw_expires_days = userhelper.password_expires_in(user.get("krbPasswordExpiration"))
@@ -111,9 +118,10 @@ else:
     </div>
 
     <br/>
-    <br/>
 
-    <a href="${tg.url('/useredit', uid=user.get('uid'))}">edit</a>
+    <input type="button"
+      onclick="document.location.href='${edit_url}'"
+      value="Edit Person" />
 
 </body>
 </html>
diff --git a/ipa-server/ipa-gui/ipagui/templates/welcome.kid b/ipa-server/ipa-gui/ipagui/templates/welcome.kid
index 0e0ad8601..d2663788a 100644
--- a/ipa-server/ipa-gui/ipagui/templates/welcome.kid
+++ b/ipa-server/ipa-gui/ipagui/templates/welcome.kid
@@ -11,6 +11,13 @@
             py:content="XML(tg_flash)"></div>
         <h1>Welcome to Free IPA</h1>
 
+        <noscript>
+        <span class="warning_message">
+        This site makes heavy use of JavaScript.<br />
+        Please enable JavaScript in your browser to make sure all pages function properly.
+        </span>
+        </noscript>
+
         <p>
           IPA is used to manage Identity, Policy, and Auditing for your
           organization.