Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($compile): Fix namespace detection for achor elements #13480

Closed
wants to merge 1 commit into from
Closed

fix($compile): Fix namespace detection for achor elements #13480

wants to merge 1 commit into from

Conversation

feelepxyz
Copy link
Contributor

Fixed a bug where anchor elements with SVG in the href attribute got incorrectly detected as svgs. Seems the detection only happens when the anchor element has a child that needs to be transcluded.

Calling node.toString() on a anchor returns the href value instead of the expected [object HTMLAnchorElement].

Added a test for this edge case.

Plunker to reproduce: http://plnkr.co/edit/OBgCS8phjTwMTFYwhk6D

@@ -1396,7 +1396,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
if (!node) {
return 'html';
} else {
return nodeName_(node) !== 'foreignobject' && node.toString().match(/SVG/) ? 'svg' : 'html';
return nodeName_(node) !== 'foreignobject' && Object.prototype.toString.call(node).match(/SVG/) ? 'svg' : 'html';
Copy link
Contributor

Choose a reason for hiding this comment

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

Just

return nodeName_(node) !== 'foreignobject' && toString.call(node).match(/SVG/) ? 'svg' : 'html';

should make it

@feelepxyz
Copy link
Contributor Author

@lgalfaso sorted. Added a commit for the same fix in ngInclude, but looking at that code: the function namespaceAdaptedClone doesn't seem to be defined anywhere in the codebase.. is this directive ever used? :)

@lgalfaso
Copy link
Contributor

@harrison the only reference of namespaceAdaptedClone is within src/ng/ngInclude.js, do not know how this is related to this PR.

One more thing, can you please name the test something like "should handle anchor elements inside svg elements"?

@lgalfaso lgalfaso self-assigned this Dec 11, 2015
@feelepxyz
Copy link
Contributor Author

@lgalfaso not related just curious if it's useless code:)

The issue happens with the ngIf/transclusion on the span which wrongly detects the parent anchor as being an svg. Changed it to transclusions should not detect parent anchor elements as SVGs, happy to change.

@lgalfaso
Copy link
Contributor

@harrison can you please squash everything into one commit? this makes things easier. Otherwise this looks good and should merge early next week

@lgalfaso lgalfaso closed this in c9e6cf9 Dec 13, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants