Bug 422121, crash sorting xml built templates, add some null checks, r=smaug,sr=jonas,a=beltzner

content/xul/templates/src/nsXULTemplateQueryProcessorStorage.cpp

@@ -448,11 +448,15 @@ nsXULTemplateQueryProcessorStorage::CompareResults(nsIXULTemplateResult* aLeft,
                                                    PRInt32* aResult)
 {
     *aResult = 0;
+    if (!aVar)
+      return NS_OK;
 
     // We're going to see if values are integers or float, to perform
     // a suitable comparison
     nsCOMPtr<nsISupports> leftValue, rightValue;
+    if (aLeft)
       aLeft->GetBindingObjectFor(aVar, getter_AddRefs(leftValue));
+    if (aRight)
       aRight->GetBindingObjectFor(aVar, getter_AddRefs(rightValue));
 
     if (leftValue && rightValue) {
@@ -496,9 +500,11 @@ nsXULTemplateQueryProcessorStorage::CompareResults(nsIXULTemplateResult* aLeft,
 
     // Values are not integers or floats, so we just compare them as simple strings
     nsAutoString leftVal;
+    if (aLeft)
       aLeft->GetBindingFor(aVar, leftVal);
 
     nsAutoString rightVal;
+    if (aRight)
       aRight->GetBindingFor(aVar, rightVal);
 
     *aResult = Compare(nsDependentString(leftVal),

content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp

@@ -331,6 +331,7 @@ nsXULTemplateQueryProcessorXML::GenerateResults(nsISupports* aDatasource,
         return NS_ERROR_INVALID_ARG;
 
     nsCOMPtr<nsIDOMNode> context;
+    if (aRef)
       aRef->GetBindingObjectFor(xmlquery->GetMemberVariable(),
                                 getter_AddRefs(context));
     if (!context)
@@ -422,15 +423,19 @@ nsXULTemplateQueryProcessorXML::CompareResults(nsIXULTemplateResult* aLeft,
                                                PRInt32* aResult)
 {
     *aResult = 0;
+    if (!aVar)
+      return NS_OK;
 
     // XXXndeakin - bug 379745
     // it would be good for this to handle other types such as integers,
     // so that sorting can be optimized for different types.
 
     nsAutoString leftVal;
+    if (aLeft)
       aLeft->GetBindingFor(aVar, leftVal);
 
     nsAutoString rightVal;
+    if (aRight)
       aRight->GetBindingFor(aVar, rightVal);
 
     // currently templates always sort case-insensitive

content/xul/templates/src/nsXULTemplateResultStorage.cpp

@@ -106,6 +106,8 @@ nsXULTemplateResultStorage::GetType(nsAString& aType)
 NS_IMETHODIMP
 nsXULTemplateResultStorage::GetBindingFor(nsIAtom* aVar, nsAString& aValue)
 {
+    NS_ENSURE_ARG_POINTER(aVar);
+
     aValue.Truncate();
     if (!mResultSet) {
         return NS_OK;
@@ -126,6 +128,8 @@ nsXULTemplateResultStorage::GetBindingFor(nsIAtom* aVar, nsAString& aValue)
 NS_IMETHODIMP
 nsXULTemplateResultStorage::GetBindingObjectFor(nsIAtom* aVar, nsISupports** aValue)
 {
+    NS_ENSURE_ARG_POINTER(aVar);
+
     if (mResultSet) {
         PRInt32 idx = mResultSet->GetColumnIndex(aVar);
         if (idx >= 0) {

content/xul/templates/src/nsXULTemplateResultXML.cpp

@@ -120,6 +120,8 @@ nsXULTemplateResultXML::GetType(nsAString& aType)
 NS_IMETHODIMP
 nsXULTemplateResultXML::GetBindingFor(nsIAtom* aVar, nsAString& aValue)
 {
+    NS_ENSURE_ARG_POINTER(aVar);
+
     // get the position of the atom in the variables table
     nsXMLBinding* binding;
 
@@ -153,6 +155,8 @@ nsXULTemplateResultXML::GetBindingFor(nsIAtom* aVar, nsAString& aValue)
 NS_IMETHODIMP
 nsXULTemplateResultXML::GetBindingObjectFor(nsIAtom* aVar, nsISupports** aValue)
 {
+    NS_ENSURE_ARG_POINTER(aVar);
+
     nsXMLBinding* binding;
     nsCOMPtr<nsIDOMNode> node;
 

toolkit/content/tests/widgets/Makefile.in

@@ -101,6 +101,7 @@ _TEST_FILES = 	test_bug360220.xul \
 		test_menu_hide.xul \
 		test_focus.xul \
 		test_tabindex.xul \
+		test_sorttemplate.xul \
 		$(NULL)
 
 ifeq (,$(filter mac cocoa,$(MOZ_WIDGET_TOOLKIT)))

toolkit/content/tests/widgets/test_sorttemplate.xul

@@ -0,0 +1,89 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="/tests/SimpleTest/test.css" type="text/css"?>
+<!--
+  XUL Widget Test for tabindex
+  -->
+<window title="tabindex" width="500" height="600"
+        onfocus="runTest()"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>  
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>  
+
+<people id="famouspeople" xmlns="">
+    <person name="Napoleon Bonaparte" gender="male"/>
+    <person name="Cleopatra" gender="female"/>
+    <person name="Julius Caesar" gender="male"/>
+    <person name="Ferdinand Magellan" gender="male"/>
+    <person name="Laura Secord" gender="female"/>
+</people>
+
+<tree id="tree" datasources="#famouspeople" ref="*" querytype="xml" flex="1">
+  <treecols>
+    <treecol label="Name" flex="1" sort="?name"/>
+    <treecol label="Gender" flex="1" sort="?gender"/>
+  </treecols>
+  <template>
+    <query/>
+    <rule>
+      <action>
+        <treechildren id="treechildren-strings">
+          <treeitem uri="?">
+            <treerow>
+              <treecell label="?name"/>
+              <treecell label="?gender"/>
+            </treerow>
+          </treeitem>
+        </treechildren>
+      </action>
+    </rule>
+  </template>
+</tree>
+
+<body xmlns="http://www.w3.org/1999/xhtml">
+<p id="display"></p>
+<div id="content" style="display: none">
+</div>
+<pre id="test">
+</pre>
+</body>
+
+<script>
+<![CDATA[
+
+SimpleTest.waitForExplicitFinish();
+
+function runTest()
+{
+  var tree = $("tree");
+  var col = tree.columns[0].element;
+  synthesizeMouse(col, 12, 2, { });
+  checkRowOrder(tree, ["Cleopatra", "Ferdinand Magellan", "Julius Caesar", "Laura Secord", "Napoleon Bonaparte"], "acsending");
+
+  synthesizeMouse(col, 12, 2, { });
+  checkRowOrder(tree, ["Napoleon Bonaparte", "Laura Secord", "Julius Caesar", "Ferdinand Magellan", "Cleopatra"], "descending");
+
+  synthesizeMouse(col, 12, 2, { });
+  checkRowOrder(tree, ["Napoleon Bonaparte", "Laura Secord", "Julius Caesar", "Ferdinand Magellan", "Cleopatra"], "natural");
+
+  SimpleTest.finish();
+}
+
+function checkRowOrder(tree, expected, testid)
+{
+  var index = 0;
+  var item = tree.firstChild.nextSibling.nextSibling.firstChild;
+  while (item && index < expected.length) {
+    if (item.firstChild.firstChild.getAttribute("label") != expected[index++])
+      break;
+    item = item.nextSibling;
+  }
+  ok(index == expected.length && !item, testid + " row order");
+}
+
+]]>
+
+</script>
+
+</window>