Skip to content
This repository has been archived by the owner on Apr 4, 2019. It is now read-only.

[BUGFIX] Adds the ability to blacklist props that should use setAttribute because of browser compliance issues #355

Merged
merged 1 commit into from
Jun 4, 2015

Conversation

jayphelps
Copy link
Contributor

fixes emberjs/ember.js#11112 and sets the stage for #353 which is the same code-path and desired outcome but different cause.

@jayphelps
Copy link
Contributor Author

Added another test.

@jayphelps
Copy link
Contributor Author

Amended with input#type as well, so we can revert emberjs/ember.js#10690 and get more natural behavior in that case (the attr value will still be the invalid type, as expected, instead of text)

@rwjblue
Copy link
Collaborator

rwjblue commented May 29, 2015

Seems good to me.

@mixonic can you review and 👍 / 👎?

@jayphelps
Copy link
Contributor Author

@mixonic ping

@rwjblue
Copy link
Collaborator

rwjblue commented Jun 3, 2015

@mixonic / @mmun - Do either of you have objections? Will merge and release tonight if I don't hear otherwise....

@stefanpenner
Copy link
Collaborator

we should add the form attribute for the appropriate tags aswell. Essentially merging this PR and https://github.com/tildeio/htmlbars/pull/352/files

@mmun
Copy link
Collaborator

mmun commented Jun 3, 2015

Seems fine.

@jayphelps
Copy link
Contributor Author

@stefanpenner I would prefer #353 since it covers more than just the form attribute case. I plan to rebase once this is merged. Thoughts?

@stefanpenner
Copy link
Collaborator

@jayphelps yes i believe so, although i don't believe it handles the type case. Or am i wrong?

@jayphelps
Copy link
Contributor Author

@stefanpenner It does not. Sorry if this is confusing. This PR handles specific cases of type for both input and button, #353 handles the form attribute in a generic way. They're in the same code path but the cause and solution of each are separate.

@jayphelps
Copy link
Contributor Author

@stefanpenner Although we could use this PR's blacklist method for form, I don't think we should because there are other cases like form that can exist in the wild, especially with w3c web components. This PR handles non-compliance to spec, where as #353 handles non-writable behaviors, regardless of whether or not it is because of compliance. i.e. I think we need both.

rwjblue added a commit that referenced this pull request Jun 4, 2015
[BUGFIX] Adds the ability to blacklist props that should use setAttribute because of browser compliance issues
@rwjblue rwjblue merged commit 06442c5 into tildeio:master Jun 4, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Binding type attribute on a button element fails in PhantomJS
4 participants