From 15944de27bbdcb2a67d4db30351d4bfde4f1f0f8 Mon Sep 17 00:00:00 2001 From: Jay Phelps Date: Thu, 4 Jun 2015 18:27:43 -0700 Subject: [PATCH] detect props that aren't settable so we fallback to setAttribute, fixes emberjs/ember.js#11221 --- packages/dom-helper/lib/prop.js | 53 +++++++++++++-- packages/dom-helper/tests/prop-test.js | 93 +++++++++++++++++++++++--- 2 files changed, 130 insertions(+), 16 deletions(-) diff --git a/packages/dom-helper/lib/prop.js b/packages/dom-helper/lib/prop.js index a78bb6a6..294722d3 100644 --- a/packages/dom-helper/lib/prop.js +++ b/packages/dom-helper/lib/prop.js @@ -15,18 +15,17 @@ export function normalizeProperty(element, attrName) { // TODO should this be an o_create kind of thing? cache = {}; for (key in element) { - key = key.toLowerCase(); + let lowerKey = key.toLowerCase(); if (isSettable(element, key)) { - cache[key] = key; + cache[lowerKey] = key; } else { - cache[key] = UNDEFINED; + cache[lowerKey] = UNDEFINED; } } propertyCaches[tagName] = cache; } - // presumes that the attrName has been lowercased. - var value = cache[attrName]; + var value = cache[attrName.toLowerCase()]; return value === UNDEFINED ? undefined : value; } @@ -52,5 +51,49 @@ function isSettable(element, attrName) { } } + // 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/prop-test.js b/packages/dom-helper/tests/prop-test.js index ed78ebac..753cda8f 100644 --- a/packages/dom-helper/tests/prop-test.js +++ b/packages/dom-helper/tests/prop-test.js @@ -1,6 +1,55 @@ -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(); + } + }; + + 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 + } + }); + + [element1, element2, element3].forEach(function (el) { + equal(normalizeProperty(el, 'form'), 'form'); + }); +}); test('returns `undefined` for special element properties that are non-compliant in certain browsers', function() { expect(2); @@ -11,17 +60,39 @@ test('returns `undefined` for special element properties that are non-compliant ]; badPairs.forEach(function(pair) { - var element = { - tagName: pair.tagName - }; - - Object.defineProperty(element, pair.key, { - set: function() { - throw new Error('I am a bad browser!'); + var proto = {}; + proto[pair.key] = { + enumerable: true, + set() { + throw new Error('I am a bad browser! '); } - }); + }; + var element = createMockElement(pair.tagName, proto); var actual = normalizeProperty(element, pair.key); equal(actual, undefined); }); -}); \ No newline at end of file +}); + +test('returns `undefined` for properties that are effectively read-only (writable=false or no setter)', function() { + expect(2); + + var element1 = createMockElement('element1', { + form: { + enumerable: true, + get() { + return null; + } + } + }); + var element2 = createMockElement('element2', { + form: { + enumerable: true, + writable: false, + value: null + } + }); + + equal(normalizeProperty(element1, 'form'), undefined); + equal(normalizeProperty(element2, 'form'), undefined); +});