Bug 662743 - Session restore should do more than restore a <select>'s selectedIndex; r=zpao
authorBellindira Castillo [:bellindira] <bellindira@appcoast.com>
Thu, 17 May 2012 23:23:35 -0600
changeset 96819 b1dc93af542d380daa03f1afabe842ba583bc721
parent 96818 a84e147b4d22cc2562c95f34420fb9c70bda0085
child 96820 49cbdb1ea4e2a4d7f44bd381d15d1125e2ac49a2
push id1439
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 20:19:22 +0000
treeherdermozilla-aurora@ea74834dccd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerszpao
bugs662743
milestone15.0a1
Bug 662743 - Session restore should do more than restore a <select>'s selectedIndex; r=zpao
browser/components/sessionstore/src/DocumentUtils.jsm
browser/components/sessionstore/test/Makefile.in
browser/components/sessionstore/test/browser_662743.js
browser/components/sessionstore/test/browser_662743_sample.html
--- a/browser/components/sessionstore/src/DocumentUtils.jsm
+++ b/browser/components/sessionstore/src/DocumentUtils.jsm
@@ -68,26 +68,26 @@ let DocumentUtils = {
             value = node.value;
             hasDefaultValue = value == node.defaultValue;
             break;
         }
       } else if (!node.multiple) {
         // <select>s without the multiple attribute are hard to determine the
         // default value, so assume we don't have the default.
         hasDefaultValue = false;
-        value = node.selectedIndex;
+        value = { selectedIndex: node.selectedIndex, value: node.value };
       } else {
         // <select>s with the multiple attribute are easier to determine the
         // default value since each <option> has a defaultSelected
         let options = Array.map(node.options, function(aOpt, aIx) {
           let oSelected = aOpt.selected;
           hasDefaultValue = hasDefaultValue && (oSelected == aOpt.defaultSelected);
-          return oSelected ? aIx : -1;
+          return oSelected ? aOpt.value : -1;
         });
-        value = options.filter(function(aIx) aIx >= 0);
+        value = options.filter(function(aIx) aIx !== -1);
       }
 
       // In order to reduce XPath generation (which is slow), we only save data
       // for form fields that have been changed. (cf. bug 537289)
       if (!hasDefaultValue) {
         if (nId) {
           ret.id[nId] = value;
         } else {
@@ -172,33 +172,51 @@ let DocumentUtils = {
       // Don't dispatch a change event for no change.
       if (aNode.checked == aValue) {
         return;
       }
       
       aNode.checked = aValue;
       eventType = "change";
     } else if (typeof aValue == "number") {
+      // handle select backwards compatibility, example { "#id" : index }
       // We saved the value blindly since selects take more work to determine
       // default values. So now we should check to avoid unnecessary events.
       if (aNode.selectedIndex == aValue) {
         return;
       }
       
-      try {
+      if (aValue < aNode.options.length) {
         aNode.selectedIndex = aValue;
         eventType = "change";
-      } catch (ex) { /* throws for invalid indices */ }
+      } 
+    } else if (aValue && aValue.selectedIndex >= 0 && aValue.value) {
+      // handle select new format
+
+      // Don't dispatch a change event for no change
+      if (aNode.options[aNode.selectedIndex].value == aValue.value) {
+        return;
+      }
+
+      // find first option with matching aValue if possible
+      for (let i = 0; i < aNode.options.length; i++) {
+        if (aNode.options[i].value == aValue.value) {
+          aNode.selectedIndex = i;
+          break;
+        }
+      }
+      eventType = "change";
     } else if (aValue && aValue.fileList && aValue.type == "file" &&
       aNode.type == "file") {
       aNode.mozSetFileNameArray(aValue.fileList, aValue.fileList.length);
       eventType = "input";
     } else if (aValue && typeof aValue.indexOf == "function" && aNode.options) {
       Array.forEach(aNode.options, function(opt, index) {
-        opt.selected = aValue.indexOf(index) > -1;
+        // don't worry about malformed options with same values
+        opt.selected = aValue.indexOf(opt.value) > -1;
         
         // Only fire the event here if this wasn't selected by default
         if (!opt.defaultSelected) {
           eventType = "change";
         }
       });
     }
 
--- a/browser/components/sessionstore/test/Makefile.in
+++ b/browser/components/sessionstore/test/Makefile.in
@@ -116,16 +116,18 @@ include $(topsrcdir)/config/rules.mk
 	browser_624727.js \
 	browser_625257.js \
 	browser_628270.js \
 	browser_635418.js \
 	browser_636279.js \
 	browser_644409-scratchpads.js \
 	browser_645428.js \
 	browser_659591.js \
+	browser_662743.js \
+	browser_662743_sample.html \
 	browser_662812.js \
 	browser_665702-state_session.js \
 	browser_682507.js \
 	browser_687710.js \
 	browser_687710_2.js \
 	browser_694378.js \
 	browser_701377.js \
 	browser_705597.js \
new file mode 100644
--- /dev/null
+++ b/browser/components/sessionstore/test/browser_662743.js
@@ -0,0 +1,128 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// This tests that session restore component does restore the right <select> option.
+// Session store should not rely only on previous user's selectedIndex, it should
+// check its value as well.
+
+function test() {
+  /** Tests selected options **/
+  waitForExplicitFinish();
+
+  let testTabCount = 0;
+  let formData = [
+  // default case
+    { },
+  // old format
+    { "#select_id" : 0 },
+    { "#select_id" : 2 },
+    // invalid index
+    { "#select_id" : 8 },
+    { "/xhtml:html/xhtml:body/xhtml:select" : 5},
+    { "/xhtml:html/xhtml:body/xhtml:select[@name='select_name']" : 6},
+
+  // new format
+    // index doesn't match value (testing an option in between (two))
+    { id:{ "select_id": {"selectedIndex":0,"value":"val2"} } },
+    // index doesn't match value (testing an invalid value)
+    { id:{ "select_id": {"selectedIndex":4,"value":"val8"} } },
+    // index doesn't match value (testing an invalid index)
+    { id:{ "select_id": {"selectedIndex":8,"value":"val5"} } },
+    // index and value match position zero
+    { id:{ "select_id": {"selectedIndex":0,"value":"val0"} }, xpath: {} },
+    // index doesn't match value (testing the last option (seven))
+    { id:{},"xpath":{ "/xhtml:html/xhtml:body/xhtml:select[@name='select_name']": {"selectedIndex":1,"value":"val7"} } },
+    // index and value match the default option "selectedIndex":3,"value":"val3"
+    { xpath: { "/xhtml:html/xhtml:body/xhtml:select[@name='select_name']" : {"selectedIndex":3,"value":"val3"} } },
+    // index matches default option however it doesn't match value
+    { id:{ "select_id": {"selectedIndex":3,"value":"val4"} } },
+
+  // combinations
+    { "#select_id" : 3, id:{ "select_id": {"selectedIndex":1,"value":"val1"} } },
+    { "#select_id" : 5, xpath: { "/xhtml:html/xhtml:body/xhtml:select[@name='select_name']" : {"selectedIndex":4,"value":"val4"} } },
+    { "/xhtml:html/xhtml:body/xhtml:select" : 5, id:{ "select_id": {"selectedIndex":1,"value":"val1"} }},
+    { "/xhtml:html/xhtml:body/xhtml:select[@name='select_name']" : 2, xpath: { "/xhtml:html/xhtml:body/xhtml:select[@name='select_name']" : {"selectedIndex":7,"value":"val7"} } }
+  ];
+
+  let expectedValues = [
+    [ "val3"], // default value
+    [ "val0"],
+    [ "val2"],
+    [ "val3"], // default value (invalid index)
+    [ "val5"],
+    [ "val6"],
+    [ "val2"],
+    [ "val3"], // default value (invalid value)
+    [ "val5"], // value is still valid (even it has an invalid index)
+    [ "val0"],
+    [ "val7"],
+    [ "val3"],
+    [ "val4"],
+    [ "val1"],
+    [ "val4"],
+    [ "val1"],
+    [ "val7"]
+  ];
+  let callback = function() {
+    testTabCount--;
+    if (testTabCount == 0) {
+      finish();
+    }
+  };
+
+  for (let i = 0; i < formData.length; i++) {
+    testTabCount++;
+    testTabRestoreData(formData[i], expectedValues[i], callback);
+  }
+}
+
+function testTabRestoreData(aFormData, aExpectedValues, aCallback) {
+  let testURL =
+    getRootDirectory(gTestPath) + "browser_662743_sample.html";
+  let tab = gBrowser.addTab(testURL);
+  let tabState = { entries: [{ url: testURL, formdata: aFormData}] };
+
+  tab.linkedBrowser.addEventListener("load", function(aEvent) {
+    tab.linkedBrowser.removeEventListener("load", arguments.callee, true);
+    ss.setTabState(tab, JSON.stringify(tabState));
+
+    tab.addEventListener("SSTabRestored", function(aEvent) {
+      tab.removeEventListener("SSTabRestored", arguments.callee);
+      let doc = tab.linkedBrowser.contentDocument;
+      let select = doc.getElementById("select_id");
+      let value = select.options[select.selectedIndex].value;
+
+      // test select options values
+      is(value, aExpectedValues[0],
+        "Select Option by selectedIndex &/or value has been restored correctly");
+
+      // clean up
+      gBrowser.removeTab(tab);
+      aCallback();
+    });
+
+    tab.addEventListener("TabClose", function(aEvent) {
+      tab.removeEventListener("TabClose", arguments.callee);
+      let restoredTabState = JSON.parse(ss.getTabState(tab));
+      let restoredFormData = restoredTabState.entries[0].formdata;
+      let selectIdFormData = restoredFormData.id.select_id;
+      let value = restoredFormData.id.select_id.value;
+
+      // test format
+      ok("id" in restoredFormData && "xpath" in restoredFormData,
+        "FormData format is valid");
+      // validate that there are no old keys
+      is(Object.keys(restoredFormData).length, 2,
+        "FormData key length is valid");
+      // test format
+      ok("selectedIndex" in selectIdFormData && "value" in selectIdFormData,
+        "select format is valid");
+      // validate that there are no old keys
+      is(Object.keys(selectIdFormData).length, 2,
+        "select_id length is valid");
+       // test set collection values
+      is(value, aExpectedValues[0],
+        "Collection has been saved correctly");
+    });
+  }, true);
+}
new file mode 100644
--- /dev/null
+++ b/browser/components/sessionstore/test/browser_662743_sample.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<title>Test 662743</title>
+
+<!-- Select events -->
+<h3>Select options</h3>
+<select id="select_id" name="select_name">
+  <option value="val0">Zero</option>
+  <option value="val1">One</option>
+  <option value="val2">Two</option>
+  <option value="val3" selected="selected">Three</option>
+  <option value="val4">Four</option>
+  <option value="val5">Five</option>
+  <option value="val6">Six</option>
+  <option value="val7">Seven</option>
+</select>
\ No newline at end of file