Skip to content

Commit 99ead47

Browse files
JoostKatscott
authored andcommitted
fix(ivy): match directives on namespaced elements (#33555)
Prior to this change, namespaced elements such as SVG elements would not participate correctly in directive matching as their namespace was not ignored, which was the case with the View Engine compiler. This led to incorrect behavior at runtime and template type checking. This commit resolved the issue by ignoring the namespace of elements and attributes like they were in View Engine. Fixes #32061 PR Close #33555
1 parent d09ad82 commit 99ead47

File tree

4 files changed

+78
-23
lines changed

4 files changed

+78
-23
lines changed

packages/compiler/src/render3/view/t2_binder.ts

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {CssSelector, SelectorMatcher} from '../../selector';
1111
import {BoundAttribute, BoundEvent, BoundText, Content, Element, Icu, Node, Reference, Template, Text, TextAttribute, Variable, Visitor} from '../r3_ast';
1212

1313
import {BoundTarget, DirectiveMeta, Target, TargetBinder} from './t2_api';
14+
import {createCssSelector} from './template';
1415
import {getAttrsForDirectiveMatching} from './util';
1516

1617

@@ -222,25 +223,10 @@ class DirectiveBinder<DirectiveT extends DirectiveMeta> implements Visitor {
222223

223224
visitTemplate(template: Template): void { this.visitElementOrTemplate('ng-template', template); }
224225

225-
visitElementOrTemplate(tag: string, node: Element|Template): void {
226+
visitElementOrTemplate(elementName: string, node: Element|Template): void {
226227
// First, determine the HTML shape of the node for the purpose of directive matching.
227228
// Do this by building up a `CssSelector` for the node.
228-
const cssSelector = new CssSelector();
229-
cssSelector.setElement(tag);
230-
231-
// Add attributes to the CSS selector.
232-
const attrs = getAttrsForDirectiveMatching(node);
233-
Object.getOwnPropertyNames(attrs).forEach((name) => {
234-
const value = attrs[name];
235-
236-
cssSelector.addAttribute(name, value);
237-
238-
// Treat the 'class' attribute specially.
239-
if (name.toLowerCase() === 'class') {
240-
const classes = value.trim().split(/\s+/g);
241-
classes.forEach(className => cssSelector.addClassName(className));
242-
}
243-
});
229+
const cssSelector = createCssSelector(elementName, getAttrsForDirectiveMatching(node));
244230

245231
// Next, use the `SelectorMatcher` to get the list of directives on the node.
246232
const directives: DirectiveT[] = [];

packages/compiler/src/render3/view/template.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,9 +1189,9 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
11891189
return args;
11901190
}
11911191

1192-
private matchDirectives(tagName: string, elOrTpl: t.Element|t.Template) {
1192+
private matchDirectives(elementName: string, elOrTpl: t.Element|t.Template) {
11931193
if (this.directiveMatcher) {
1194-
const selector = createCssSelector(tagName, getAttrsForDirectiveMatching(elOrTpl));
1194+
const selector = createCssSelector(elementName, getAttrsForDirectiveMatching(elOrTpl));
11951195
this.directiveMatcher.match(
11961196
selector, (cssSelector, staticType) => { this.directives.add(staticType); });
11971197
}
@@ -1762,15 +1762,18 @@ export class BindingScope implements LocalResolver {
17621762
/**
17631763
* Creates a `CssSelector` given a tag name and a map of attributes
17641764
*/
1765-
function createCssSelector(tag: string, attributes: {[name: string]: string}): CssSelector {
1765+
export function createCssSelector(
1766+
elementName: string, attributes: {[name: string]: string}): CssSelector {
17661767
const cssSelector = new CssSelector();
1768+
const elementNameNoNs = splitNsName(elementName)[1];
17671769

1768-
cssSelector.setElement(tag);
1770+
cssSelector.setElement(elementNameNoNs);
17691771

17701772
Object.getOwnPropertyNames(attributes).forEach((name) => {
1773+
const nameNoNs = splitNsName(name)[1];
17711774
const value = attributes[name];
17721775

1773-
cssSelector.addAttribute(name, value);
1776+
cssSelector.addAttribute(nameNoNs, value);
17741777
if (name.toLowerCase() === 'class') {
17751778
const classes = value.trim().split(/\s+/);
17761779
classes.forEach(className => cssSelector.addClassName(className));

packages/compiler/test/render3/view/binding_spec.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,26 @@ describe('t2 binding', () => {
6464
expect(directives[0].name).toBe('NgFor');
6565
});
6666

67+
it('should match directives on namespaced elements', () => {
68+
const template = parseTemplate('<svg><text dir>SVG</text></svg>', '', {});
69+
const matcher = new SelectorMatcher<DirectiveMeta>();
70+
matcher.addSelectables(CssSelector.parse('text[dir]'), {
71+
name: 'Dir',
72+
exportAs: null,
73+
inputs: {},
74+
outputs: {},
75+
isComponent: false,
76+
});
77+
const binder = new R3TargetBinder(matcher);
78+
const res = binder.bind({template: template.nodes});
79+
const svgNode = template.nodes[0] as a.Element;
80+
const textNode = svgNode.children[0] as a.Element;
81+
const directives = res.getDirectivesOfNode(textNode) !;
82+
expect(directives).not.toBeNull();
83+
expect(directives.length).toBe(1);
84+
expect(directives[0].name).toBe('Dir');
85+
});
86+
6787
it('should not match directives intended for an element on a microsyntax template', () => {
6888
const template = parseTemplate('<div *ngFor="let item of items" dir></div>', '', {});
6989
const binder = new R3TargetBinder(makeSelectorMatcher());

packages/core/test/acceptance/directive_spec.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {CommonModule} from '@angular/common';
10-
import {Component, Directive, EventEmitter, Output, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
10+
import {Component, Directive, ElementRef, EventEmitter, Output, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
1111
import {Input} from '@angular/core/src/metadata';
1212
import {TestBed} from '@angular/core/testing';
1313
import {By} from '@angular/platform-browser';
@@ -260,6 +260,52 @@ describe('directives', () => {
260260
expect(calls).toEqual(['MyDir.ngOnInit']);
261261
});
262262

263+
it('should match directives on elements with namespace', () => {
264+
const calls: string[] = [];
265+
266+
@Directive({selector: 'svg[dir]'})
267+
class MyDir {
268+
constructor(private el: ElementRef) {}
269+
ngOnInit() { calls.push(`MyDir.ngOnInit: ${this.el.nativeElement.tagName}`); }
270+
}
271+
272+
@Component({
273+
selector: `my-comp`,
274+
template: `<svg dir><text dir></text></svg>`,
275+
})
276+
class MyComp {
277+
}
278+
279+
TestBed.configureTestingModule({declarations: [MyDir, MyComp]});
280+
const fixture = TestBed.createComponent(MyComp);
281+
fixture.detectChanges();
282+
283+
expect(calls).toEqual(['MyDir.ngOnInit: svg']);
284+
});
285+
286+
it('should match directives on descendant elements with namespace', () => {
287+
const calls: string[] = [];
288+
289+
@Directive({selector: 'text[dir]'})
290+
class MyDir {
291+
constructor(private el: ElementRef) {}
292+
ngOnInit() { calls.push(`MyDir.ngOnInit: ${this.el.nativeElement.tagName}`); }
293+
}
294+
295+
@Component({
296+
selector: `my-comp`,
297+
template: `<svg dir><text dir></text></svg>`,
298+
})
299+
class MyComp {
300+
}
301+
302+
TestBed.configureTestingModule({declarations: [MyDir, MyComp]});
303+
const fixture = TestBed.createComponent(MyComp);
304+
fixture.detectChanges();
305+
306+
expect(calls).toEqual(['MyDir.ngOnInit: text']);
307+
});
308+
263309
it('should match directives when the node has "class", "style" and a binding', () => {
264310
const logs: string[] = [];
265311

0 commit comments

Comments
 (0)