Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(platform-browser): setAttribute should work with xmlns namespace #14874

Merged
merged 1 commit into from Mar 23, 2017
Merged

fix(platform-browser): setAttribute should work with xmlns namespace #14874

merged 1 commit into from Mar 23, 2017

Conversation

DzmitryShylovich
Copy link
Contributor

Closes #14865

vicb
vicb reviewed Mar 4, 2017
View reviewed changes
modules/@angular/platform-browser/src/dom/dom_renderer.ts Outdated
@@ -149,7 +149,13 @@ class DefaultDomRendererV2 implements RendererV2 {

setAttribute(el: any, name: string, value: string, namespace?: string): void {
if (namespace) {
el.setAttributeNS(NAMESPACE_URIS[namespace], namespace + ':' + name, value);
const attrNs = NAMESPACE_URIS[namespace];
Copy link
Contributor

@vicb vicb Mar 4, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • what about adding the xmlns NS ? (This should be the list of all supported NS).
  • missing tests

@DzmitryShylovich
Copy link
Contributor Author

@vicb added uri for xmlns namespace + tests PTAL

@DzmitryShylovich DzmitryShylovich changed the title fix(platform-browser): DomRendererV2 shouldn't throw when namespace uri is undefined fix(platform-browser): setAttribute should work with xmlns namespace Mar 4, 2017
@DzmitryShylovich
Copy link
Contributor Author

@IgorMinar can you take a look please? it's a regression from 2.x

@patrickmichalina
Copy link

Can this be merged in before the next RC? It is preventing any migration to 4.0.0.RC for my projects.

@SamVerschueren
Copy link
Contributor

Same here, have to downgrade back to 2.x after noticing this in one of my views.

tbosch
tbosch approved these changes Mar 23, 2017
View reviewed changes
Copy link
Contributor

@tbosch tbosch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is a good change!

@tbosch tbosch added the action: merge The PR is ready for merge by the caretaker label Mar 23, 2017
@vicb vicb mentioned this pull request Mar 23, 2017
Closed
@vicb vicb merged commit 92084f2 into angular:master Mar 23, 2017
@SamVerschueren
Copy link
Contributor

Thanks @DzmitryShylovich for fixing it and thanks @IgorMinar and @vicb for following up! 🍻

@DzmitryShylovich DzmitryShylovich deleted the gh/14865 branch March 23, 2017 20:48
sis0k0 added a commit to NativeScript/nativescript-angular that referenced this pull request Mar 24, 2017
@sis0k0
fix(renderer): support namespaced attributes
2fccdd3 
Platform attributes (such as :ios and :android) are passed as
namespaces to the renderer in Angular 4.
Caused by: angular/angular#14874.
sis0k0 added a commit to NativeScript/nativescript-angular that referenced this pull request Mar 24, 2017
@sis0k0
fix(renderer): support namespaced attributes
62927e0 
Platform attributes (such as :ios and :android) are passed as
namespaces to the renderer in Angular 4.
Caused by: angular/angular#14874.
@sis0k0 sis0k0 mentioned this pull request Mar 24, 2017
Merged
sis0k0 added a commit to NativeScript/nativescript-angular that referenced this pull request Mar 24, 2017
@sis0k0
fix(renderer): support namespaced attributes (#719)
184f887 
Platform attributes (such as :ios and :android) are passed as
namespaces to the renderer in Angular 4.
Caused by: angular/angular#14874.
hdeshev added a commit to NativeScript/nativescript-angular that referenced this pull request Mar 28, 2017
@hdeshev
fix(renderer): support namespaced attributes (#719)
be3f954 
Platform attributes (such as :ios and :android) are passed as
namespaces to the renderer in Angular 4.
Caused by: angular/angular#14874.
hdeshev added a commit to NativeScript/nativescript-angular that referenced this pull request Mar 28, 2017
@hdeshev
fix(renderer): support namespaced attributes (#719)
9b5b413 
Platform attributes (such as :ios and :android) are passed as
namespaces to the renderer in Angular 4.
Caused by: angular/angular#14874.
asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
@DzmitryShylovich
fix(platform-browser): setAttribute should work with xmlns namespace (a…
3adb1ab 
…ngular#14874)

Closes angular#14865
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@DzmitryShylovich @juleskremer
fix(platform-browser): setAttribute should work with xmlns namespace (a…
d8177e0 
…ngular#14874)

Closes angular#14865
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vicb vicb vicb left review comments

@tbosch tbosch tbosch approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Failed to execute 'setAttributeNS' on 'Element': '' - on xmlns:v="..."
6 participants
@DzmitryShylovich @patrickmichalina @SamVerschueren @vicb @tbosch @googlebot