Skip to content

Commit

Permalink
Merge pull request Polymer#105 from Polymer/platform_88_take2
Browse files Browse the repository at this point in the history
blacklist problematic DOM property names
  • Loading branch information
Scott J. Miles committed Oct 8, 2014
2 parents 254e77a + 82d04a2 commit a23ea1e
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 0 deletions.
41 changes: 41 additions & 0 deletions src/declaration/properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,37 @@
if (publish) {
// transcribe `publish` entries onto own prototype
this.requireProperties(publish, prototype, base);
// warn and remove accessor names that are broken on some browsers
this.filterInvalidAccessorNames(publish);
// construct map of lower-cased property names
prototype._publishLC = this.lowerCaseMap(publish);
}
var computed = prototype.computed;
if (computed) {
// warn and remove accessor names that are broken on some browsers
this.filterInvalidAccessorNames(computed);
}
},
// Publishing/computing a property where the name might conflict with a
// browser property is not currently supported to help users of Polymer
// avoid browser bugs:
//
// https://code.google.com/p/chromium/issues/detail?id=43394
// https://bugs.webkit.org/show_bug.cgi?id=49739
//
// We can lift this restriction when those bugs are fixed.
filterInvalidAccessorNames: function(propertyNames) {
for (var name in propertyNames) {
// Check if the name is in our blacklist.
if (this.propertyNameBlacklist[name]) {
console.warn('Cannot define property "' + name + '" for element "' +
this.name + '" because it has the same name as an HTMLElement ' +
'property, and not all browsers support overriding that. ' +
'Consider giving it a different name.');
// Remove the invalid accessor from the list.
delete propertyNames[name];
}
}
},
//
// `name: value` entries in the `publish` object may need to generate
Expand Down Expand Up @@ -181,6 +209,19 @@
}
}
}
},
// This list contains some property names that people commonly want to use,
// but won't work because of Chrome/Safari bugs. It isn't an exhaustive
// list. In particular it doesn't contain any property names found on
// subtypes of HTMLElement (e.g. name, value). Rather it attempts to catch
// some common cases.
propertyNameBlacklist: {
children: 1,
'class': 1,
id: 1,
hidden: 1,
style: 1,
title: 1,
}
};

Expand Down
22 changes: 22 additions & 0 deletions test/js/attrs.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,25 @@ htmlSuite('attributes-declarative', function() {
htmlTest('html/attr-mustache.html');
htmlTest('html/prop-attr-reflection.html');
});

suite('attributes', function() {
var assert = chai.assert;

test('override dom accessor', function() {
var p = document.createElement('polymer-element');
p.setAttribute('name', 'test-override-dom-accessor');
p.setAttribute('attributes', 'title');
p.setAttribute('noscript', '');
p.init();
// Because Chrome and Safari are busted...
// https://code.google.com/p/chromium/issues/detail?id=43394
// https://bugs.webkit.org/show_bug.cgi?id=49739
//
// ... Polymer doesn't currently support accessor names used by the DOM.
var t = document.createElement('test-override-dom-accessor');
t.title = 123;
assert.strictEqual(t.title, '123');
// The 'title' property was not recorded as published.
assert.deepEqual(p.prototype.publish, {});
});
});

0 comments on commit a23ea1e

Please sign in to comment.