From 04587a33c5eca98fc0aa346d7ce35a5616de83db Mon Sep 17 00:00:00 2001 From: crisbeto Date: Wed, 5 Jun 2019 14:39:01 +0200 Subject: [PATCH] fix(ivy): DebugNode.attributes not preserving attribute name casing (#30864) In Ivy the `DebugNode.attributes` is populated directly from the DOM, but the problem is that the browser will lowercase all attribute names. These changes preserve the case by first going through the `TNode.attrs`, populating the map with the case-sensitive names and saving a reference to the lower case name. Afterwards when we're going through the attributes from the DOM, we can check whether we've mapped the attribute by its case-sensitive name already. This PR resolves FW-1358. PR Close #30864 --- packages/core/src/debug/debug_node.ts | 45 +++++++++++++++-- packages/core/test/debug/debug_node_spec.ts | 55 ++++++++++++++++++++- 2 files changed, 95 insertions(+), 5 deletions(-) diff --git a/packages/core/src/debug/debug_node.ts b/packages/core/src/debug/debug_node.ts index bc15842731..61f138ee6c 100644 --- a/packages/core/src/debug/debug_node.ts +++ b/packages/core/src/debug/debug_node.ts @@ -276,13 +276,50 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme get attributes(): {[key: string]: string | null;} { const attributes: {[key: string]: string | null;} = {}; const element = this.nativeElement; - if (element) { - const eAttrs = element.attributes; - for (let i = 0; i < eAttrs.length; i++) { - const attr = eAttrs[i]; + + if (!element) { + return attributes; + } + + const context = loadLContext(element); + const lView = context.lView; + const tNodeAttrs = (lView[TVIEW].data[context.nodeIndex] as TNode).attrs; + const lowercaseTNodeAttrs: string[] = []; + + // For debug nodes we take the element's attribute directly from the DOM since it allows us + // to account for ones that weren't set via bindings (e.g. ViewEngine keeps track of the ones + // that are set through `Renderer2`). The problem is that the browser will lowercase all names, + // however since we have the attributes already on the TNode, we can preserve the case by going + // through them once, adding them to the `attributes` map and putting their lower-cased name + // into an array. Afterwards when we're going through the native DOM attributes, we can check + // whether we haven't run into an attribute already through the TNode. + if (tNodeAttrs) { + let i = 0; + while (i < tNodeAttrs.length) { + const attrName = tNodeAttrs[i]; + + // Stop as soon as we hit a marker. We only care about the regular attributes. Everything + // else will be handled below when we read the final attributes off the DOM. + if (typeof attrName !== 'string') break; + + const attrValue = tNodeAttrs[i + 1]; + attributes[attrName] = attrValue as string; + lowercaseTNodeAttrs.push(attrName.toLowerCase()); + + i += 2; + } + } + + const eAttrs = element.attributes; + for (let i = 0; i < eAttrs.length; i++) { + const attr = eAttrs[i]; + // Make sure that we don't assign the same attribute both in its + // case-sensitive form and the lower-cased one from the browser. + if (lowercaseTNodeAttrs.indexOf(attr.name) === -1) { attributes[attr.name] = attr.value; } } + return attributes; } diff --git a/packages/core/test/debug/debug_node_spec.ts b/packages/core/test/debug/debug_node_spec.ts index cbec33a9ca..4f318f0b1a 100644 --- a/packages/core/test/debug/debug_node_spec.ts +++ b/packages/core/test/debug/debug_node_spec.ts @@ -7,7 +7,7 @@ */ -import {Component, DebugNode, Directive, ElementRef, EmbeddedViewRef, EventEmitter, HostBinding, Injectable, Input, NO_ERRORS_SCHEMA, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core'; +import {Component, DebugNode, Directive, ElementRef, EmbeddedViewRef, EventEmitter, HostBinding, Injectable, Input, NO_ERRORS_SCHEMA, Renderer2, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core'; import {ComponentFixture, TestBed, async} from '@angular/core/testing'; import {By} from '@angular/platform-browser/src/dom/debug/by'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; @@ -736,5 +736,58 @@ class TestCmptWithPropBindings { ]); }); + it('should preserve the attribute case in DebugNode.attributes', () => { + @Component({selector: 'my-icon', template: ''}) + class Icon { + @Input() svgIcon: any = ''; + } + @Component({template: ``}) + class App { + } + + TestBed.configureTestingModule({declarations: [App, Icon]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + const element = fixture.debugElement.children[0]; + + // Assert that the camel-case attribute is correct. + expect(element.attributes.svgIcon).toBe('test'); + + // Make sure that we somehow didn't preserve the native lower-cased value. + expect(element.attributes.svgicon).toBeFalsy(); + }); + + + it('should include namespaced attributes in DebugNode.attributes', () => { + @Component({ + template: `
`, + }) + class Comp { + } + + TestBed.configureTestingModule({declarations: [Comp]}); + const fixture = TestBed.createComponent(Comp); + fixture.detectChanges(); + + expect(fixture.debugElement.query(By.css('div')).attributes['xlink:href']).toBe('foo'); + }); + + it('should include attributes added via Renderer2 in DebugNode.attributes', () => { + @Component({ + template: '
', + }) + class Comp { + constructor(public renderer: Renderer2) {} + } + + TestBed.configureTestingModule({declarations: [Comp]}); + const fixture = TestBed.createComponent(Comp); + const div = fixture.debugElement.query(By.css('div')); + + fixture.componentInstance.renderer.setAttribute(div.nativeElement, 'foo', 'bar'); + + expect(div.attributes.foo).toBe('bar'); + }); + }); }