From 7e6f536cf532b9c5dae1482ccc41c137fde3254e Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Wed, 25 Feb 2015 15:30:37 +0100 Subject: [PATCH] fix(compiler): properly bind to properties that don't have matching attr name Fixes #619 Closes #783 --- .../pipeline/element_binder_builder.js | 31 ++++++++++++---- modules/angular2/src/facade/dom.dart | 8 +++- modules/angular2/src/facade/dom.es6 | 2 + .../test/core/compiler/integration_spec.js | 37 +++++++++++++++++++ .../pipeline/element_binder_builder_spec.js | 21 +++++++++++ 5 files changed, 91 insertions(+), 8 deletions(-) diff --git a/modules/angular2/src/core/compiler/pipeline/element_binder_builder.js b/modules/angular2/src/core/compiler/pipeline/element_binder_builder.js index 71dc650b04..ab8c815c67 100644 --- a/modules/angular2/src/core/compiler/pipeline/element_binder_builder.js +++ b/modules/angular2/src/core/compiler/pipeline/element_binder_builder.js @@ -1,5 +1,5 @@ import {int, isPresent, isBlank, Type, BaseException, StringWrapper, RegExpWrapper, isString, stringify} from 'angular2/src/facade/lang'; -import {Element, DOM} from 'angular2/src/facade/dom'; +import {Element, DOM, attrToPropMap} from 'angular2/src/facade/dom'; import {ListWrapper, List, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection'; import {reflector} from 'angular2/src/reflection/reflection'; @@ -91,11 +91,19 @@ function roleSetter(element:Element, value) { } } +// special mapping for cases where attribute name doesn't match property name +var attrToProp = StringMapWrapper.merge({ + "inner-html": "innerHTML", + "readonly": "readOnly", + "tabindex": "tabIndex", +}, attrToPropMap); + // tells if an attribute is handled by the ElementBinderBuilder step export function isSpecialProperty(propName:string) { - return StringWrapper.startsWith(propName, ARIA_PREFIX) - || StringWrapper.startsWith(propName, CLASS_PREFIX) - || StringWrapper.startsWith(propName, STYLE_PREFIX); + return StringWrapper.startsWith(propName, ARIA_PREFIX) + || StringWrapper.startsWith(propName, CLASS_PREFIX) + || StringWrapper.startsWith(propName, STYLE_PREFIX) + || StringMapWrapper.contains(attrToProp, propName); } /** @@ -176,10 +184,14 @@ export class ElementBinderBuilder extends CompileStep { setterFn = classSetterFactory(StringWrapper.substring(property, CLASS_PREFIX.length)); } else if (StringWrapper.startsWith(property, STYLE_PREFIX)) { styleParts = StringWrapper.split(property, DOT_REGEXP); - styleSuffix = styleParts.length > 2 ? ListWrapper.get(styleParts, 2) : ''; + styleSuffix = styleParts.length > 2 ? ListWrapper.get(styleParts, 2) : ''; setterFn = styleSetterFactory(ListWrapper.get(styleParts, 1), styleSuffix); - } else if (DOM.hasProperty(compileElement.element, property)) { - setterFn = reflector.setter(property); + } else { + property = this._resolvePropertyName(property); + //TODO(pk): special casing innerHtml, see: https://github.com/angular/angular/issues/789 + if (DOM.hasProperty(compileElement.element, property) || StringWrapper.equals(property, 'innerHtml')) { + setterFn = reflector.setter(property); + } } if (isPresent(setterFn)) { @@ -236,4 +248,9 @@ export class ElementBinderBuilder extends CompileStep { var parts = StringWrapper.split(bindConfig, RegExpWrapper.create("\\|")); return ListWrapper.map(parts, (s) => s.trim()); } + + _resolvePropertyName(attrName:string) { + var mappedPropName = StringMapWrapper.get(attrToProp, attrName); + return isPresent(mappedPropName) ? mappedPropName : attrName; + } } diff --git a/modules/angular2/src/facade/dom.dart b/modules/angular2/src/facade/dom.dart index b96260caa1..380eb059e9 100644 --- a/modules/angular2/src/facade/dom.dart +++ b/modules/angular2/src/facade/dom.dart @@ -17,7 +17,8 @@ export 'dart:html' show InputElement, AnchorElement, Text, - window; + window, + attrToPropMap; // TODO(tbosch): Is there a builtin one? Why is Dart // removing unknown elements by default? @@ -36,6 +37,11 @@ void gc() { final identitySanitizer = new IdentitySanitizer(); +// override JS logic of attribute to property mapping +var attrToPropMap = { + "inner-html": "innerHtml" +}; + class DOM { static query(String selector) => document.querySelector(selector); diff --git a/modules/angular2/src/facade/dom.es6 b/modules/angular2/src/facade/dom.es6 index c3c8dcbfed..ffd6d0aca0 100644 --- a/modules/angular2/src/facade/dom.es6 +++ b/modules/angular2/src/facade/dom.es6 @@ -16,6 +16,8 @@ export var gc = window.gc ? () => window.gc() : () => null; export var CssRule = window.CSSRule; export var CssKeyframesRule = window.CSSKeyframesRule; +export var attrToPropMap = {}; + export class DOM { static query(selector) { return document.querySelector(selector); diff --git a/modules/angular2/test/core/compiler/integration_spec.js b/modules/angular2/test/core/compiler/integration_spec.js index bbb95b31ea..b645e5b0b8 100644 --- a/modules/angular2/test/core/compiler/integration_spec.js +++ b/modules/angular2/test/core/compiler/integration_spec.js @@ -102,6 +102,41 @@ export function main() { }); }); + it('should consume binding to propery names where attr name and property name do not match', (done) => { + tplResolver.setTemplate(MyComp, new Template({inline: '
'})); + + compiler.compile(MyComp).then((pv) => { + createView(pv); + + cd.detectChanges(); + expect(view.nodes[0].tabIndex).toEqual(0); + + ctx.ctxNumProp = 5; + cd.detectChanges(); + expect(view.nodes[0].tabIndex).toEqual(5); + + done(); + }); + }); + + it('should consume binding to inner-html', (done) => { + tplResolver.setTemplate(MyComp, new Template({inline: '
'})); + + compiler.compile(MyComp).then((pv) => { + createView(pv); + + ctx.ctxProp = 'Some HTML'; + cd.detectChanges(); + expect(DOM.getInnerHTML(view.nodes[0])).toEqual('Some HTML'); + + ctx.ctxProp = 'Some other
HTML
'; + cd.detectChanges(); + expect(DOM.getInnerHTML(view.nodes[0])).toEqual('Some other
HTML
'); + + done(); + }); + }); + it('should consume directive watch expression change.', (done) => { var tpl = '
' + @@ -490,8 +525,10 @@ class PushBasedComp { @Component() class MyComp { ctxProp:string; + ctxNumProp; constructor() { this.ctxProp = 'initial value'; + this.ctxNumProp = 0; } } diff --git a/modules/angular2/test/core/compiler/pipeline/element_binder_builder_spec.js b/modules/angular2/test/core/compiler/pipeline/element_binder_builder_spec.js index 3dba4875dc..1475d1bb06 100644 --- a/modules/angular2/test/core/compiler/pipeline/element_binder_builder_spec.js +++ b/modules/angular2/test/core/compiler/pipeline/element_binder_builder_spec.js @@ -196,6 +196,27 @@ export function main() { expect(view.nodes[0].hidden).toEqual(false); }); + it('should bind element properties where attr name and prop name do not match', () => { + var propertyBindings = MapWrapper.createFromStringMap({ + 'tabindex': 'prop1' + }); + var pipeline = createPipeline({propertyBindings: propertyBindings}); + var results = pipeline.process(el('
')); + var pv = results[0].inheritedProtoView; + + expect(pv.elementBinders[0].hasElementPropertyBindings).toBe(true); + + instantiateView(pv); + + evalContext.prop1 = 1; + changeDetector.detectChanges(); + expect(view.nodes[0].tabIndex).toEqual(1); + + evalContext.prop1 = 0; + changeDetector.detectChanges(); + expect(view.nodes[0].tabIndex).toEqual(0); + }); + it('should bind to aria-* attributes when exp evaluates to strings', () => { var propertyBindings = MapWrapper.createFromStringMap({ 'aria-label': 'prop1'