diff --git a/packages/dom-helper/lib/prop.js b/packages/dom-helper/lib/prop.js index 695c1474..fff4a319 100644 --- a/packages/dom-helper/lib/prop.js +++ b/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 with 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; +} diff --git a/packages/dom-helper/tests/dom-helper-test.js b/packages/dom-helper/tests/dom-helper-test.js index 9222fb5d..eeebe16e 100644 --- a/packages/dom-helper/tests/dom-helper-test.js +++ b/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, '
'); + + // 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(){ diff --git a/packages/dom-helper/tests/prop-test.js b/packages/dom-helper/tests/prop-test.js index 5318340b..320d1541 100644 --- a/packages/dom-helper/tests/prop-test.js +++ b/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); });