Merge pull request #2218 from knockout/2218-value-update-select

Select element doesn't update properly when combined with if binding
knockout · Apr 9, 2017 · 4391eb9 · 4391eb9
2 parents 79d3f5f + bd6e508
commit 4391eb9
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 10 deletions.
diff --git a/spec/defaultBindings/valueBehaviors.js b/spec/defaultBindings/valueBehaviors.js
@@ -360,6 +360,11 @@ describe('Binding: Value', function() {
             observable("");
             expect(testNode.childNodes[0].selectedIndex).toEqual(0);
 
+            // Also check that the selection doesn't change later (see https://github.com/knockout/knockout/issues/2218)
+            waits(10);
+            runs(function() {
+                expect(testNode.childNodes[0].selectedIndex).toEqual(0);
+            });
         });
 
         it('Should display the caption when the model value changes to undefined, null, or \"\" when options specified directly', function() {

diff --git a/src/binding/defaultBindings/value.js b/src/binding/defaultBindings/value.js
@@ -94,20 +94,11 @@ ko.bindingHandlers['value'] = {
                 if (valueHasChanged) {
                     if (tagName === "select") {
                         var allowUnset = allBindings.get('valueAllowUnset');
-                        var applyValueAction = function () {
-                            ko.selectExtensions.writeValue(element, newValue, allowUnset);
-                        };
-                        applyValueAction();
-
+                        ko.selectExtensions.writeValue(element, newValue, allowUnset);
                         if (!allowUnset && newValue !== ko.selectExtensions.readValue(element)) {
                             // If you try to set a model value that can't be represented in an already-populated dropdown, reject that change,
                             // because you're not allowed to have a model value that disagrees with a visible UI selection.
                             ko.dependencyDetection.ignore(ko.utils.triggerEvent, null, [element, "change"]);
-                        } else {
-                            // Workaround for IE6 bug: It won't reliably apply values to SELECT nodes during the same execution thread
-                            // right after you've changed the set of OPTION nodes on it. So for that node type, we'll schedule a second thread
-                            // to apply the value as well.
-                            ko.utils.setTimeout(applyValueAction, 0);
                         }
                     } else {
                         ko.selectExtensions.writeValue(element, newValue);

diff --git a/src/binding/selectExtensions.js b/src/binding/selectExtensions.js
@@ -55,6 +55,14 @@
                     }
                     if (allowUnset || selection >= 0 || (value === undefined && element.size > 1)) {
                         element.selectedIndex = selection;
+                        if (ko.utils.ieVersion === 6) {
+                            // Workaround for IE6 bug: It won't reliably apply values to SELECT nodes during the same execution thread
+                            // right after you've changed the set of OPTION nodes on it. So for that node type, we'll schedule a second thread
+                            // to apply the value as well.
+                            ko.utils.setTimeout(function () {
+                                element.selectedIndex = selection;
+                            }, 0);
+                        }
                     }
                     break;
                 default: