fix(core): consistently use namespace short name rather than URI (#44766)

atscott · AndrewKushnir · commit fb9163ae501e · 2022-01-21T11:19:30.000-08:00
`Renderer2` APIs expect to be called with the namespace name rather than the namespace URI. Rather than passing around the URI and having to account for different calling contexts, this change consistently uses the namespace short names. Importantly, the URI was only used in `component_ref.ts` `create` (because `getNamespace returned the URIs`) and `createElementNode` in `node_manipulation.ts` (because `getNamespaceUri` also used the URIs). In contrast, attributes would use the _short names instead of URIs_ (see `setUpAttributes` in `attrs_utils.ts`). These names are pulled directly from the attribute, i.e. `xhtml:href` and not converted to URI. This dichotomy is confusing and unnecessary. The change here aligns the two approaches in order to provide consistently throughout the system. This relates to #44766 because the `createElementNode` was calling the `AnimationRenderer.createElement` which delegates to the `ServerRenderer`, which in turn was only set up to expect short names. As a result, the `NAMESPACE_URIS` lookup failed and `Domino` created the `svg` as a regular `Element` which does not have a `styles` property. resolves #44766 PR Close #44766
diff --git a/packages/core/src/render3/namespaces.ts b/packages/core/src/render3/namespaces.ts
@@ -6,5 +6,13 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-export const SVG_NAMESPACE = 'http://www.w3.org/2000/svg';
-export const MATH_ML_NAMESPACE = 'http://www.w3.org/1998/MathML/';
+export const SVG_NAMESPACE = 'svg';
+export const SVG_NAMESPACE_URI = 'http://www.w3.org/2000/svg';
+export const MATH_ML_NAMESPACE = 'math';
+export const MATH_ML_NAMESPACE_URI = 'http://www.w3.org/1998/MathML/';
+
+export function getNamespaceUri(namespace: string): string|null {
+  const name = namespace.toLowerCase();
+  return name === SVG_NAMESPACE ? SVG_NAMESPACE_URI :
+                                  (name === MATH_ML_NAMESPACE ? MATH_ML_NAMESPACE_URI : null);
+}
diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts
@@ -25,6 +25,7 @@ import {isProceduralRenderer, ProceduralRenderer3, Renderer3, unusedValueExportT
 import {RComment, RElement, RNode, RText} from './interfaces/renderer_dom';
 import {isLContainer, isLView} from './interfaces/type_checks';
 import {CHILD_HEAD, CLEANUP, DECLARATION_COMPONENT_VIEW, DECLARATION_LCONTAINER, DestroyHookData, FLAGS, HookData, HookFn, HOST, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, T_HOST, TVIEW, TView, TViewType, unusedValueExportToPlacateAjd as unused5} from './interfaces/view';
+import {getNamespaceUri} from './namespaces';
 import {assertTNodeType} from './node_assert';
 import {profiler, ProfilerEvent} from './profiler';
 import {getLViewParent} from './util/view_traversal_utils';
@@ -132,8 +133,9 @@ export function createElementNode(
   if (isProceduralRenderer(renderer)) {
     return renderer.createElement(name, namespace);
   } else {
-    return namespace === null ? renderer.createElement(name) :
-                                renderer.createElementNS(namespace, name);
+    const namespaceUri = namespace !== null ? getNamespaceUri(namespace) : null;
+    return namespaceUri === null ? renderer.createElement(name) :
+                                   renderer.createElementNS(namespaceUri, name);
   }
 }
 
diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts
@@ -542,10 +542,6 @@ describe('runtime i18n', () => {
       TestBed.configureTestingModule({
         providers: [
           {provide: DOCUMENT, useFactory: _document, deps: []},
-          // TODO(FW-811): switch back to default server renderer (i.e. remove the line
-          // below) once it starts to support Ivy namespace format (URIs) correctly. For
-          // now, use `DomRenderer` that supports Ivy namespace format.
-          {provide: RendererFactory2, useClass: DomRendererFactory2}
         ],
       });
     });
diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts
@@ -197,10 +197,6 @@ describe('ViewContainerRef', () => {
             declarations: [TestComp, SvgComp, MathMLComp],
             providers: [
               {provide: DOCUMENT, useFactory: _document, deps: []},
-              // TODO(FW-811): switch back to default server renderer (i.e. remove the line below)
-              // once it starts to support Ivy namespace format (URIs) correctly. For now, use
-              // `DomRenderer` that supports Ivy namespace format.
-              {provide: RendererFactory2, useClass: DomRendererFactory2}
             ],
           });
           const fixture = TestBed.createComponent(TestComp);
diff --git a/packages/platform-browser/src/dom/dom_renderer.ts b/packages/platform-browser/src/dom/dom_renderer.ts
@@ -17,6 +17,7 @@ export const NAMESPACE_URIS: {[ns: string]: string} = {
   'xlink': 'http://www.w3.org/1999/xlink',
   'xml': 'http://www.w3.org/XML/1998/namespace',
   'xmlns': 'http://www.w3.org/2000/xmlns/',
+  'math': 'http://www.w3.org/1998/MathML/',
 };
 
 const COMPONENT_REGEX = /%COMP%/g;
@@ -144,9 +145,7 @@ class DefaultDomRenderer2 implements Renderer2 {
 
   createElement(name: string, namespace?: string): any {
     if (namespace) {
-      // In cases where Ivy (not ViewEngine) is giving us the actual namespace, the look up by key
-      // will result in undefined, so we just return the namespace here.
-      return document.createElementNS(NAMESPACE_URIS[namespace] || namespace, name);
+      return document.createElementNS(NAMESPACE_URIS[namespace], name);
     }
 
     return document.createElement(name);
@@ -199,8 +198,6 @@ class DefaultDomRenderer2 implements Renderer2 {
   setAttribute(el: any, name: string, value: string, namespace?: string): void {
     if (namespace) {
       name = namespace + ':' + name;
-      // TODO(FW-811): Ivy may cause issues here because it's passing around
-      // full URIs for namespaces, therefore this lookup will fail.
       const namespaceUri = NAMESPACE_URIS[namespace];
       if (namespaceUri) {
         el.setAttributeNS(namespaceUri, name, value);
@@ -214,15 +211,10 @@ class DefaultDomRenderer2 implements Renderer2 {
 
   removeAttribute(el: any, name: string, namespace?: string): void {
     if (namespace) {
-      // TODO(FW-811): Ivy may cause issues here because it's passing around
-      // full URIs for namespaces, therefore this lookup will fail.
       const namespaceUri = NAMESPACE_URIS[namespace];
       if (namespaceUri) {
         el.removeAttributeNS(namespaceUri, name);
       } else {
-        // TODO(FW-811): Since ivy is passing around full URIs for namespaces
-        // this could result in properties like `http://www.w3.org/2000/svg:cx="123"`,
-        // which is wrong.
         el.removeAttribute(`${namespace}:${name}`);
       }
     } else {
diff --git a/packages/platform-server/src/server_renderer.ts b/packages/platform-server/src/server_renderer.ts
@@ -72,8 +72,6 @@ class DefaultServerRenderer2 implements Renderer2 {
   createElement(name: string, namespace?: string, debugInfo?: any): any {
     if (namespace) {
       const doc = this.document || getDOM().getDefaultDocument();
-      // TODO(FW-811): Ivy may cause issues here because it's passing around
-      // full URIs for namespaces, therefore this lookup will fail.
       return doc.createElementNS(NAMESPACE_URIS[namespace], name);
     }
 
@@ -131,8 +129,6 @@ class DefaultServerRenderer2 implements Renderer2 {
 
   setAttribute(el: any, name: string, value: string, namespace?: string): void {
     if (namespace) {
-      // TODO(FW-811): Ivy may cause issues here because it's passing around
-      // full URIs for namespaces, therefore this lookup will fail.
       el.setAttributeNS(NAMESPACE_URIS[namespace], namespace + ':' + name, value);
     } else {
       el.setAttribute(name, value);
@@ -141,8 +137,6 @@ class DefaultServerRenderer2 implements Renderer2 {
 
   removeAttribute(el: any, name: string, namespace?: string): void {
     if (namespace) {
-      // TODO(FW-811): Ivy may cause issues here because it's passing around
-      // full URIs for namespaces, therefore this lookup will fail.
       el.removeAttributeNS(NAMESPACE_URIS[namespace], name);
     } else {
       el.removeAttribute(name);
diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts
@@ -193,7 +193,11 @@ class SVGServerModule {
 
 @Component({
   selector: 'app',
-  template: `<div [@myAnimation]="state">{{text}}</div>`,
+  template: `
+  <div [@myAnimation]="state">
+    <svg *ngIf="true"></svg>
+    {{text}}
+  </div>`,
   animations: [trigger(
       'myAnimation',
       [
