Don't set the first option as selected in select tag with size attribute (#14242)

kulek1 · nhunzaker · commit ab5fe174c69e · 2019-03-14T14:40:09.000-07:00
* Set 'size' attribute to select tag if it occurs before appending options

* Add comment about why size is assigned on select create. Tests

I added some more clarification for why size must be set on select
element creation:

- In the source code
- In the DOM test fixture
- In a unit test

* Use let, not const in select tag stub assignment
diff --git a/fixtures/dom/src/components/fixtures/selects/index.js b/fixtures/dom/src/components/fixtures/selects/index.js
@@ -202,6 +202,34 @@ class SelectFixture extends React.Component {
             </select>
           </div>
         </TestCase>
+
+        <TestCase
+          title="A select with the size attribute should not set first option as selected"
+          relatedIssues="14239"
+          introducedIn="16.0.0">
+          <TestCase.ExpectedResult>
+            No options should be selected.
+          </TestCase.ExpectedResult>
+
+          <div className="test-fixture">
+            <select size="3">
+              <option>0</option>
+              <option>1</option>
+              <option>2</option>
+            </select>
+          </div>
+
+          <p className="footnote">
+            <b>Notes:</b> This happens if <code>size</code> is assigned after
+            options are selected. The select element picks the first item by
+            default, then it is expanded to show more options when{' '}
+            <code>size</code> is assigned, preserving the default selection.
+          </p>
+          <p className="footnote">
+            This was introduced in React 16.0.0 when options were added before
+            select attribute assignment.
+          </p>
+        </TestCase>
       </FixtureSet>
     );
   }
diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js
@@ -362,6 +362,32 @@ describe('ReactDOMSelect', () => {
     expect(node.options[2].selected).toBe(true); // gorilla
   });
 
+  it('does not select an item when size is initially set to greater than 1', () => {
+    const stub = (
+      <select size="2">
+        <option value="monkey">A monkey!</option>
+        <option value="giraffe">A giraffe!</option>
+        <option value="gorilla">A gorilla!</option>
+      </select>
+    );
+    const container = document.createElement('div');
+    const select = ReactDOM.render(stub, container);
+
+    expect(select.options[0].selected).toBe(false);
+    expect(select.options[1].selected).toBe(false);
+    expect(select.options[2].selected).toBe(false);
+
+    // Note: There is an inconsistency between JSDOM and Chrome where
+    // Chrome reports an empty string when no value is selected for a
+    // single-select with a size greater than 0. JSDOM reports the first
+    // value
+    //
+    // This assertion exists only for clarity of JSDOM behavior:
+    expect(select.value).toBe('monkey'); // "" in Chrome
+    // Despite this, the selection index is correct:
+    expect(select.selectedIndex).toBe(-1);
+  });
+
   it('should remember value when switching to uncontrolled', () => {
     let stub = (
       <select value={'giraffe'} onChange={noop}>
diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js
@@ -419,14 +419,25 @@ export function createElement(
       // See discussion in https://github.com/facebook/react/pull/6896
       // and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
       domElement = ownerDocument.createElement(type);
-      // Normally attributes are assigned in `setInitialDOMProperties`, however the `multiple`
-      // attribute on `select`s needs to be added before `option`s are inserted. This prevents
-      // a bug where the `select` does not scroll to the correct option because singular
-      // `select` elements automatically pick the first item.
+      // Normally attributes are assigned in `setInitialDOMProperties`, however the `multiple` and `size`
+      // attributes on `select`s needs to be added before `option`s are inserted.
+      // This prevents:
+      // - a bug where the `select` does not scroll to the correct option because singular
+      //  `select` elements automatically pick the first item #13222
+      // - a bug where the `select` set the first item as selected despite the `size` attribute #14239
       // See https://github.com/facebook/react/issues/13222
-      if (type === 'select' && props.multiple) {
+      // and https://github.com/facebook/react/issues/14239
+      if (type === 'select') {
         const node = ((domElement: any): HTMLSelectElement);
-        node.multiple = true;
+        if (props.multiple) {
+          node.multiple = true;
+        } else if (props.size) {
+          // Setting a size greater than 1 causes a select to behave like `multiple=true`, where
+          // it is possible that no option is selected.
+          //
+          // This is only necessary when a select in "single selection mode".
+          node.size = props.size;
+        }
       }
     }
   } else {
