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

Revert SVG passthrough and expand whitelist #6243

Merged
merged 10 commits into from Mar 11, 2016

Conversation

Projects
None yet
5 participants
@zpao
Member

zpao commented Mar 10, 2016

Based on some discussions, this reverts a few commits that changed our SVG behavior to pass through all attributes to the node directly. After a lot of discussion that came in after the RC (see #6211), we mostly agree that while the move to pass through was a great effort and ultimately might be where we should go with attributes, it left us in an inconsistent state for our HTML and SVG behaviors. That's not awesome as there are already a number of things that must be learned for React and throwing in another inconsitency doesn't help. So we'll backpedal for now but still continue to provide full SVG support by going back to our previous whitelist and expanding it.

The list here was compiled with (a version of) https://gist.github.com/zpao/4f5ce1d96b11b8a70e07 which scrapes MDN for all of its SVG attributes.

Some additional changes in here as an effort to reduce bytesize. Specifically, since there are a lot of things in SVG that need to be mapped, we're duplicating the keys a lot. We can take some shortcuts to build the config with code so I did that. I also replaced all the nulls in our config with a zero. null was a placeholder value that we weren't using (only using it to check masks and it's 0 anyway for that).

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Mar 10, 2016

Member

zpao@22aacce is really the most obvious change. It's after the reverts and before any optimization attempts so is purely the new attributes over 0.14.

Member

zpao commented Mar 10, 2016

zpao@22aacce is really the most obvious change. It's after the reverts and before any optimization attempts so is purely the new attributes over 0.14.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Mar 11, 2016

@zpao updated the pull request.

facebook-github-bot commented Mar 11, 2016

@zpao updated the pull request.

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Mar 11, 2016

Member

ok. We'll revisit if we need to in the future.

Member

sebmarkbage commented Mar 11, 2016

ok. We'll revisit if we need to in the future.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Mar 11, 2016

@zpao updated the pull request.

facebook-github-bot commented Mar 11, 2016

@zpao updated the pull request.

zpao added a commit that referenced this pull request Mar 11, 2016

Merge pull request #6243 from zpao/fuck-svg-2
Revert SVG passthrough and expand whitelist

@zpao zpao merged commit ec25297 into facebook:master Mar 11, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@zpao zpao added this to the 15.0 milestone Mar 11, 2016

@ronald11126

This comment has been minimized.

Show comment
Hide comment
@ronald11126
@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Mar 15, 2016

Member

We will also need to update the docs before the release, right?
The list in the docs is now incomplete.

Member

gaearon commented Mar 15, 2016

We will also need to update the docs before the release, right?
The list in the docs is now incomplete.

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Mar 16, 2016

Member

Correct, we've done this in the past for the other releases and we just
take the config file & do some manipulation with vim, then paste into docs.

On Tuesday, March 15, 2016, Dan Abramov notifications@github.com wrote:

We will also need to update the docs before the release, right?
The list in the docs is now incomplete.


You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub
#6243 (comment)

Member

zpao commented Mar 16, 2016

Correct, we've done this in the past for the other releases and we just
take the config file & do some manipulation with vim, then paste into docs.

On Tuesday, March 15, 2016, Dan Abramov notifications@github.com wrote:

We will also need to update the docs before the release, right?
The list in the docs is now incomplete.


You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub
#6243 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment