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.

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

Override namespace with xmlns attribute #1825

Merged
merged 4 commits into from
May 3, 2017
Merged

Override namespace with xmlns attribute #1825

merged 4 commits into from
May 3, 2017

Conversation

SamuelTilly
Copy link
Contributor

Fixes issue #1819 foreignObject inside SVG

Fixes issue #1819 foreignObject inside SVG
@dead-claudia
Copy link
Member

@tivac @lhorie Any idea why this isn't triggering a Travis run?

@tivac
Copy link
Contributor

tivac commented May 1, 2017

Nope, seems real weird though.

@dead-claudia
Copy link
Member

I suspect it's probably a GH webhook issue, or else it'd at least show something here (e.g. an infinitely pending Travis build).

@lhorie
Copy link
Member

lhorie commented May 1, 2017

yeah, not seeing anything on travis. Can you maybe try to push an empty commit to see if it kicks in again? @SamuelTilly

@SamuelTilly
Copy link
Contributor Author

SamuelTilly commented May 1, 2017

looks like it got rejected by travis "abuse detected: known offender (request looked fishy)" https://travis-ci.org/lhorie/mithril.js/requests, looks like it was originally triggered from this " abuse detected: user page on GitHub gives 404 (request looked fishy) " weird.

@dead-claudia
Copy link
Member

@lhorie So I guess you get to contact Travis, then... 😦

@SamuelTilly
Copy link
Contributor Author

Yeah, got flagged for having an old link on my personal page, never bothered to remove it. contacted the support now.

@dead-claudia
Copy link
Member

@SamuelTilly Could you tell us here once it's resolved?

@SamuelTilly
Copy link
Contributor Author

@isiahmeadows will do :)

@SamuelTilly
Copy link
Contributor Author

@isiahmeadows @lhorie @tivac, all resolved now, silly commit history tho xD

render/render.js Outdated
}

return ns = vnode.attrs && vnode.attrs.xmlns || ns[vnode.tag]
Copy link
Member

Choose a reason for hiding this comment

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

return ns = vnode.attrs smells fishy...

Also, you could put the ns object in the parent scope to avoid re-creating it every time getNameSpace() is called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

valid points, i updated the pr :)

@pygy
Copy link
Member

pygy commented May 1, 2017

Some feedback sent... also, feel free to squash locally and force-push (alternatively, we can also squash the commits from the GH UI when merging).

@dead-claudia dead-claudia merged commit de4433c into MithrilJS:next May 3, 2017
@dead-claudia
Copy link
Member

LGTM (hence the merge)

@SamuelTilly SamuelTilly deleted the patch-foreignobject branch May 3, 2017 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants