Bug 662743 - Session restore should do more than restore a <select>'s selectedIndex; r=zpao

browser/components/sessionstore/src/DocumentUtils.jsm

@@ -73,16 +73,16 @@ let DocumentUtils = {
         // <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
@@ -177,23 +177,41 @@ let DocumentUtils = {
       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) {

browser/components/sessionstore/test/Makefile.in

@@ -121,6 +121,8 @@ _BROWSER_TEST_FILES = \
 	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/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);
+}

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>