Skip to content
This repository has been archived by the owner on Apr 4, 2019. It is now read-only.

Commit

Permalink
[Bugfix] fix props first logic
Browse files Browse the repository at this point in the history
* for exceptions input.form, input.list, button.type always use setAttributes
* for form.action always escape
* always assign handlers to props, even if the case appears strange
* refactor normalizeProperty to return an object that clearly indicates type: & normalized:
  • Loading branch information
stefanpenner committed Jul 3, 2015
1 parent 0522699 commit 3bdd2e6
Show file tree
Hide file tree
Showing 9 changed files with 152 additions and 76 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,4 @@
"qunit": "^0.7.2",
"rsvp": "~3.0.6"
}
}
}
4 changes: 2 additions & 2 deletions packages/dom-helper/lib/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,8 @@ prototype.setProperty = function(element, name, value, namespace) {
}
}
} else {
var normalized = normalizeProperty(element, name);
if (normalized) {
var { normalized , type } = normalizeProperty(element, name);
if (type === 'prop') {
element[normalized] = value;
} else {
if (isAttrRemovalValue(value)) {
Expand Down
108 changes: 58 additions & 50 deletions packages/dom-helper/lib/prop.js
Original file line number Diff line number Diff line change
@@ -1,63 +1,71 @@
export function isAttrRemovalValue(value) {
return value === null || value === undefined;
}
/*
*
* @method normalizeProperty
* @param element {HTMLElement}
* @param slotName {String}
* @returns {Object} { name, type }
*/
export function normalizeProperty(element, slotName) {
var type, normalized;

function UNDEFINED() {}

// TODO should this be an o_create kind of thing?
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;
} else {
cache[key] = UNDEFINED;
}
if (slotName in element) {
normalized = slotName;
type = 'prop';
} else {
var lower = slotName.toLowerCase();
if (lower in element) {
type = 'prop';
normalized = lower;
} else {
type = 'attr';
normalized = slotName;
}
propertyCaches[tagName] = cache;
}

// presumes that the attrName has been lowercased.
var value = cache[attrName];
return value === UNDEFINED ? undefined : value;
if (type === 'prop' && preferAttr(element.tagName, normalized)) {
type = 'attr';
}

return { normalized, type };
}

// 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 = [{
// properties that MUST be set as attributes, due to:
// * browser bug
// * strange spec outlier
var ATTR_OVERRIDES = {

// 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'
}];

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;
}
}
BUTTON: { type: true, form: true },

INPUT: {
// TODO: remove when IE8 is droped
// 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
list: true,
// Some version of IE (like IE9) actually throw an exception
// if you set input.type = 'something-unknown'
type: true,
form: true
},

// element.form is actually a legitimate readOnly property, that is to be
// mutated, but must be mutated by setAttribute...
SELECT: { form: true },
OPTION: { form: true },
TEXTAREA: { form: true },
LABEL: { form: true },
FIELDSET: { form: true },
LEGEND: { form: true },
OBJECT: { form: true }
};

return true;
function preferAttr(tagName, propName) {
var tag = ATTR_OVERRIDES[tagName.toUpperCase()];
return tag && tag[propName.toLowerCase()] || false;
}
4 changes: 2 additions & 2 deletions packages/dom-helper/tests/dom-helper-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,14 @@ test('#setProperty removes attr with undefined', function(){

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 node = dom.createElement(pair.tagName);
var setAttribute = node.setAttribute;

node.setAttribute = function(attrName, value) {
Expand Down
87 changes: 74 additions & 13 deletions packages/dom-helper/tests/prop-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,88 @@ 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(3);
test('type.attr, for element props that for one reason or another need to be treated as attrs', function() {
expect(12);

var badPairs = [
{ tagName: 'BUTTON', key: 'type' },
{ tagName: 'INPUT', key: 'type' },
{ tagName: 'INPUT', key: 'list' }
];
[
{ tagName: 'TEXTAREA', key: 'form' },
{ tagName: 'BUTTON', key: 'type' },
{ tagName: 'INPUT', key: 'type' },
{ tagName: 'INPUT', key: 'list' },
{ tagName: 'INPUT', key: 'form' },
{ tagName: 'OPTION', key: 'form' },
{ tagName: 'INPUT', key: 'form' },
{ tagName: 'BUTTON', key: 'form' },
{ tagName: 'LABEL', key: 'form' },
{ tagName: 'FIELDSET', key: 'form' },
{ tagName: 'LEGEND', key: 'form' },
{ tagName: 'OBJECT', key: 'form' }
].forEach((pair) => {
var element = {
tagName: pair.tagName
};

Object.defineProperty(element, pair.key, {
set() { throw new Error('I am a bad browser!'); }
});

badPairs.forEach(function(pair) {
deepEqual(normalizeProperty(element, pair.key), {
normalized: pair.key,
type: 'attr'
}, ` ${pair.tagName}.${pair.key}`);
});
});

var TAG_EVENT_PAIRS = [
{ tagName: 'form', key: 'onsubmit' },
{ tagName: 'form', key: 'onSubmit' },
{ tagName: 'form', key: 'ONSUBMIT' },
{ tagName: 'video', key: 'canplay' },
{ tagName: 'video', key: 'canPlay' },
{ tagName: 'video', key: 'CANPLAY' }
];

test('type.eventHandlers should all be props: Chrome', function() {
expect(6);
TAG_EVENT_PAIRS.forEach((pair) => {
var element = {
tagName: pair.tagName
};

Object.defineProperty(element, pair.key, {
set: function() {
throw new Error('I am a bad browser!');
}
set() { },
get() { }
});

var actual = normalizeProperty(element, pair.key);
equal(actual, undefined);
deepEqual(normalizeProperty(element, pair.key), {
normalized: pair.key,
type: 'prop'
}, ` ${pair.tagName}.${pair.key}`);
});
});


test('type.eventHandlers should all be props: Safari style (which has screwed up stuff)', function() {
expect(24);

TAG_EVENT_PAIRS.forEach((pair) => {
var parent = {
tagName: pair.tagName
};

Object.defineProperty(parent, pair.key, {
set: undefined,
get: undefined
});

var element = Object.create(parent);

ok(Object.getOwnPropertyDescriptor(element, pair.key) === undefined, 'ensure we mimic silly safari');
ok(Object.getOwnPropertyDescriptor(parent, pair.key).set === undefined, 'ensure we mimic silly safari');

var { normalized, type } = normalizeProperty(element, pair.key);

equal(normalized, pair.key, `normalized: ${pair.tagName}.${pair.key}`);
equal(type, 'prop', `type: ${pair.tagName}.${pair.key}`);
});
});
1 change: 1 addition & 0 deletions packages/htmlbars-compiler/tests/dirtying-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ test("helper calls follow the normal dirtying rules", function() {
test("attribute nodes follow the normal dirtying rules", function() {
var template = compile("<div class={{value}}>hello</div>");
var object = { value: "world" };

var result = template.render(object, env);

equalTokens(result.fragment, "<div class='world'>hello</div>", "Initial render");
Expand Down
10 changes: 6 additions & 4 deletions packages/morph-attr/lib/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,22 @@ function AttrMorph(element, attrName, domHelper, namespace) {
this.rendered = false;
this._renderedInitially = false;

var normalizedAttrName = normalizeProperty(this.element, attrName);

if (this.namespace) {
this._update = updateAttributeNS;
this._get = getAttributeNS;
this.attrName = attrName;
} else {
if (element.namespaceURI === svgNamespace || attrName === 'style' || !normalizedAttrName) {
var { normalized, type } = normalizeProperty(this.element, attrName);

if (element.namespaceURI === svgNamespace || attrName === 'style' || type === 'attr') {
this._update = updateAttribute;
this._get = getAttribute;
this.attrName = attrName;
this.attrName = normalized ;
} else {
this._update = updateProperty;
this._get = getProperty;
this.attrName = normalizedAttrName;
this.attrName = normalized ;
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions packages/morph-attr/lib/sanitize-attribute-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ var badTags = {
'LINK': true,
'IMG': true,
'IFRAME': true,
'BASE': true
'BASE': true,
'FORM': true
};

var badTagsForDataURI = {
Expand All @@ -21,7 +22,8 @@ var badTagsForDataURI = {
export var badAttributes = {
'href': true,
'src': true,
'background': true
'background': true,
'action': true
};

var badAttributesForDataURI = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,15 @@ var badTags = [
'LINK',
'IMG',
'IFRAME',
'BASE'
'BASE',
'FORM',
];

var badAttributes = [
'href',
'src',
'background'
'background',
'action'
];

var someIllegalProtocols = [
Expand Down

0 comments on commit 3bdd2e6

Please sign in to comment.