detect props that aren't settable so we fallback to setAttribute, fixes

emberjs/ember.js#11221

packages/dom-helper/lib/prop.js

@@ -9,17 +9,16 @@ export var propertyCaches = {};
 
 export function normalizeProperty(element, attrName) {
   var tagName = element.tagName;
-  var key, cachedAttrName;
   var cache = propertyCaches[tagName];
   if (!cache) {
     // TODO should this be an o_create kind of thing?
     cache = {};
-    for (cachedAttrName in element) {
-      key = cachedAttrName.toLowerCase();
-      if (isSettable(element, cachedAttrName)) {
-        cache[key] = cachedAttrName;
+    for (let key in element) {
+      let lowerKey = key.toLowerCase();
+      if (isSettable(element, key)) {
+        cache[lowerKey] = key;
       } else {
-        cache[key] = UNDEFINED;
+        cache[lowerKey] = UNDEFINED;
       }
     }
     propertyCaches[tagName] = cache;
@@ -30,34 +29,111 @@ export function normalizeProperty(element, attrName) {
   return value === UNDEFINED ? undefined : value;
 }
 
-// elements with a property that does not conform to the spec in certain
-// browsers. In these cases, we'll end up using setAttribute instead
-var badPairs = [{
-  // phantomjs < 2.0 lets you set it as a prop but won't reflect it
-  // back to the attribute. button.getAttribute('type') === null
-  tagName: 'BUTTON',
-  propName: 'type'
-}, {
-  // Some version of IE (like IE9) actually throw an exception
-  // if you set input.type = 'something-unknown'
-  tagName: 'INPUT',
-  propName: 'type'
-}, {
-  // Some versions of IE (IE8) throw an exception when setting
-  // `input.list = 'somestring'`:
-  // https://github.com/emberjs/ember.js/issues/10908
-  // https://github.com/emberjs/ember.js/issues/11364
-  tagName: 'INPUT',
-  propName: 'list'
-}];
+const NATIVE_TAGS_WITH_HYPHENS = 'ANNOTATION-XML COLOR-PROFILE FONT-FACE FONT-FACE-SRC FONT-FACE-URI FONT-FACE-FORMAT FONT-FACE-NAME MISSING-GLYPH'.split(' ');
 
+/**
+  elements with a property that does not conform to the spec in certain
+  browsers. In these cases, we'll end up using setAttribute instead
+ */
+const BLACKLIST = {
+  /*
+    input.type
+      Some versions of IE (like IE9) actually throw an exception
+      if you set input.type = 'something-unknown'
+      https://github.com/emberjs/ember.js/issues/10860
+      https://github.com/emberjs/ember.js/pull/10690
+
+    input.list
+      Some versions of IE (like IE8) throw an exception when setting
+      `input.list = 'somestring'`:
+      https://github.com/emberjs/ember.js/issues/10908
+      https://github.com/emberjs/ember.js/issues/11364
+
+    input.form
+      Like the rest of the form-aware elements, this property does not have
+      an actual setter so is effectively read-only.
+      https://github.com/emberjs/ember.js/issues/11221
+   */
+  INPUT: ['type', 'list', 'form'],
+  /*
+    button.type
+      phantomjs < 2.0 lets you set it as a prop but won't reflect it
+      back to the attribute. button.getAttribute('type') === null
+      https://github.com/emberjs/ember.js/issues/11112
+   */
+  BUTTON: ['type', 'form'],
+  SELECT: ['form'],
+  OPTION: ['form'],
+  TEXTAREA: ['form'],
+  LABEL: ['form'],
+  FIELDSET: ['form'],
+  LEGEND: ['form'],
+  OBJECT: ['form']
+};
+
+/**
+  Checking whether an elements property isn't as simple as it might seem.
+  Primarily we need to deal with browser spec differences or non-compliance, as
+  well as Custom Elements. Be very mindful changing this section as there are
+  many many edge cases.
+ */
 function isSettable(element, attrName) {
-  for (let i = 0, l = badPairs.length; i < l; i++) {
-    let pair = badPairs[i];
-    if (pair.tagName === element.tagName && pair.propName === attrName) {
+  const { tagName } = element;
+  const blackListedProps = BLACKLIST[tagName];
+
+  if (blackListedProps && blackListedProps.indexOf(attrName) !== -1) {
+    return false;
+  }
+
+  // Custom Elements require a hyphen, but don't count the list of native
+  // elements which also contain one.
+  if (tagName.indexOf('-') !== -1 && NATIVE_TAGS_WITH_HYPHENS.indexOf(tagName) === -1) {
+    // This is the ideal way to check if a property is settable, but can only
+    // be trusted on Custom Elements because of browser differences.
+    // Properties can be effectively read-only two ways.
+    // If actually marked as writable = false, an exception is thrown if you attempt
+    // to assign. If it's simply missing a setter, it silently just doesn't
+    // assign anything. Both cases we will defer to setAttribute instead
+    var desc = getPropertyDescriptor(element, attrName);
+    if (!desc) { return true; }
+    if (desc.writable === false || !desc.hasOwnProperty('value') && typeof desc.set !== 'function') {
       return false;
     }
   }
 
   return true;
 }
+
+// Polyfill :(
+const getPrototypeOf = (function() {
+  let fn = Object.getPrototypeOf;
+
+  if (!fn) {
+    /* jshint ignore:start */
+    if (typeof 'test'.__proto__ === 'object') {
+      fn = function getPrototypeOf(obj) {
+        return obj.__proto__; 
+      };
+    } else {
+      // IE8
+      fn = function getPrototypeOf(obj) {
+        return obj.constructor.prototype;
+      };
+    }
+    /* jshint ignore:end */
+  }
+
+  return fn;
+})();
+
+const { getOwnPropertyDescriptor } = Object;
+
+// Walks up the chain to find the desc by name
+function getPropertyDescriptor(obj, key) {
+  let proto = obj, desc;
+  while (proto && !(desc = getOwnPropertyDescriptor(proto, key))) {
+    proto = getPrototypeOf(proto);
+  }
+
+  return desc;
+}

packages/dom-helper/tests/dom-helper-test.js

@@ -184,6 +184,22 @@ test('#setProperty', function(){
   node = dom.createElement('div');
   dom.setProperty(node, 'style', 'color: red;');
   equalHTML(node, '<div style="color: red;"></div>');
+
+  // Tests for browser quirk corrections
+
+  [ 
+    { tagName: 'button', key: 'type' },
+    { tagName: 'input', key: 'type' },
+    { tagName: 'input', key: 'list' }
+  ]
+    .forEach(function (item) {
+      node = dom.createElement(item.tagName);
+      dom.setProperty(node, item.key, 'x-foo-bar');
+      // The property may or may not be set depending
+      // on the browser. We only care that the attribute
+      // is actually set, in this test
+      equal(node.getAttribute(item.key), 'x-foo-bar');
+    });
 });
 
 test('#setProperty removes attr with undefined', function(){

packages/dom-helper/tests/prop-test.js

@@ -1,28 +1,110 @@
-import { normalizeProperty } from 'dom-helper/prop';
+import { normalizeProperty, propertyCaches } from 'dom-helper/prop';
 
-QUnit.module('dom-helper prop');
+function createMockElement(tagName, props = {}) {
+  props.tagName = {
+    configurable: true,
+    enumerable: true,
+    get() {
+      return tagName.toUpperCase();
+    }
+  };
 
-test('returns `undefined` for special element properties that are non-compliant in certain browsers', function() {
+  function MockElement() {}
+  Object.defineProperties(MockElement.prototype, props);
+  return new MockElement();
+}
+
+QUnit.module('dom-helper prop', {
+  teardown() {
+    for (let key in propertyCaches) {
+      delete propertyCaches[key];
+    }
+  }
+});
+
+test('returns normalized property name for the typical cases', function() {
   expect(3);
+
+  var element1 = createMockElement('element1');
+  element1.form = null;
+  var element2 = createMockElement('element2', {
+    form: {
+      enumerable: true,
+      get() {
+        return null;
+      },
+      set() {
+        return null;
+      }
+    }
+  });
+  var element3 = createMockElement('element3', {
+    form: {
+      enumerable: true,
+      writable: true,
+      value: null
+    }
+  });
 
-  var badPairs = [
-    { tagName: 'BUTTON', key: 'type' },
-    { tagName: 'INPUT', key: 'type' },
-    { tagName: 'INPUT', key: 'list' }
-  ];
+  [element1, element2, element3].forEach(function (el) {
+    equal(normalizeProperty(el, 'form'), 'form');
+  });
+});
 
-  badPairs.forEach(function(pair) {
-    var element = {
-      tagName: pair.tagName
-    };
+test('returns `undefined` for special element properties that are non-compliant in certain browsers', function() {
+  expect(12);
 
-    Object.defineProperty(element, pair.key, {
-      set: function() {
-        throw new Error('I am a bad browser!');
-      }
-    });
+  const blacklist = {
+    INPUT: ['type', 'list', 'form'],
+    BUTTON: ['type', 'form'],
+    SELECT: ['form'],
+    OPTION: ['form'],
+    TEXTAREA: ['form'],
+    LABEL: ['form'],
+    FIELDSET: ['form'],
+    LEGEND: ['form'],
+    OBJECT: ['form']
+  };
+
+  for (let tagName in blacklist) {
+    let badProps = blacklist[tagName];
+
+    for (let i = 0, l = badProps.length; i < l; i++) {
+      let key = badProps[i];
+      let proto = {};
+      proto[key] = {
+        enumerable: true,
+        set() {
+          throw new Error('I am a bad browser! ');
+        }
+      };
+      let element = createMockElement(tagName, proto);
+
+      let actual = normalizeProperty(element, key);
+      equal(actual, undefined);
+    }
+  }
+});
 
-    var actual = normalizeProperty(element, pair.key);
-    equal(actual, undefined);
+test('returns `undefined` for Custom Element properties that are effectively read-only (writable=false or no setter)', function() {
+  expect(2);
+
+  var element1 = createMockElement('x-foo', {
+    suchwow: {
+      enumerable: true,
+      get() {
+        return null;
+      }
+    }
   });
+  var element2 = createMockElement('x-bar', {
+    suchwow: {
+      enumerable: true,
+      writable: false,
+      value: null
+    }
+  });
+
+  equal(normalizeProperty(element1, 'suchwow'), undefined);
+  equal(normalizeProperty(element2, 'suchwow'), undefined);
 });