Skip to content

Commit

Permalink
[BUGFIX] Adds the ability to blacklist props that should use setAttri…
Browse files Browse the repository at this point in the history
…bute because of browser compliance issues, fixes emberjs/ember.js#11112
  • Loading branch information
jayphelps committed May 29, 2015
1 parent 325de7c commit 120869a
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 2 deletions.
37 changes: 35 additions & 2 deletions packages/dom-helper/lib/prop.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ export function isAttrRemovalValue(value) {
return value === null || value === undefined;
}

function UNDEFINED() {}

// TODO should this be an o_create kind of thing?
export var propertyCaches = {};

Expand All @@ -13,11 +15,42 @@ export function normalizeProperty(element, attrName) {
// TODO should this be an o_create kind of thing?
cache = {};
for (key in element) {
cache[key.toLowerCase()] = key;
key = key.toLowerCase();
if (isSettable(element, key)) {
cache[key] = key;
} else {
cache[key] = UNDEFINED;
}
}
propertyCaches[tagName] = cache;
}

// presumes that the attrName has been lowercased.
return cache[attrName];
var value = cache[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'
}];

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) {
return false;
}
}

return true;
}
30 changes: 30 additions & 0 deletions packages/dom-helper/tests/dom-helper-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,36 @@ test('#setProperty removes attr with undefined', function(){
equalHTML(node, '<div></div>', 'undefined attribute removes the attribute');
});

test('#setProperty uses setAttribute for special non-compliant element props', function() {
expect(6);

var badPairs = [
{ tagName: 'button', key: 'type', value: 'submit', selfClosing: false },
{ tagName: 'input', key: 'type', value: 'x-not-supported', selfClosing: true }
];

badPairs.forEach(function(pair) {
var node = dom.createElement(pair.tagName);
var setAttribute = node.setAttribute;

node.setAttribute = function(attrName, value) {
equal(attrName, pair.key, 'setAttribute called with correct attrName');
equal(value, pair.value, 'setAttribute called with correct value');
return setAttribute.call(this, attrName, value);
};

dom.setProperty(node, pair.key, pair.value);

// e.g. <button type="submit"></button>
var expected = '<' + pair.tagName + ' ' + pair.key + '="' + pair.value + '">';
if (pair.selfClosing === false) {
expected += '</' + pair.tagName + '>';
}

equalHTML(node, expected, 'output html is correct');
});
});

test('#addClasses', function(){
var node = dom.createElement('div');
dom.addClasses(node, ['super-fun']);
Expand Down
27 changes: 27 additions & 0 deletions packages/dom-helper/tests/prop-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { normalizeProperty } from 'dom-helper/prop';

QUnit.module('dom-helper prop');

test('returns `undefined` for special element properties that are non-compliant in certain browsers', function() {
expect(2);

var badPairs = [
{ tagName: 'BUTTON', key: 'type' },
{ tagName: 'INPUT', key: 'type' }
];

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 actual = normalizeProperty(element, pair.key);
equal(actual, undefined);
});
});

0 comments on commit 120869a

Please sign in to comment.