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

Ensure attributeBindings work when legacy view addon is disabled. #12318

Merged
merged 2 commits into from
Sep 10, 2015

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Sep 10, 2015

When the _Ember.ENV._LEGACY_VIEW_SUPPORT flag is falsey, the view stream is not setup as of 6963750. This resulted in anything added in attributeBindings being undefined (because the attribute binding system was essentially using href=view.href instead of href=this.href or just href=href).

You might thing (as I did initially): "HOW CAN WE NOT HAVE A SINGLE TEST THAT CHECKS FOR AN ATTRIBUTE VALUE?!?!?!?", and the answer is somewhat more disconcerting. We currently run all tests with the legacy view and controller flags set to true. :(

Future test refactoring efforts will be needed to fix that mistake.


Fixes #12302.

@nathanhammond
Copy link
Member

I reviewed the stream implementation in great detail and didn't catch that this is what was resulting in the attributes not being present. We've seen similar behavior from liquid-fire–I'll confirm that this is a "likely-complete" fix tomorrow.

When the `_Ember.ENV._LEGACY_VIEW_SUPPORT` flag is falsey, the `view`
stream is not setup as of 6963750. This resulted in anything added in
`attributeBindings` being `undefined` (because the attribute binding
system was essentially using `href=view.href` instead of
`href=this.href` or just `href=href`).

You might thing (as I did initially): "HOW CAN WE NOT HAVE A SINGLE TEST
THAT CHECKS FOR AN ATTRIBUTE VALUE?!?!?!?", and the answer is somewhat
more disconcerting.  We currently run all tests with the legacy view and
controller flags set to `true`. :(

Future test refactoring efforts will be needed to fix that mistake.
@rwjblue
Copy link
Member Author

rwjblue commented Sep 10, 2015

Updated to avoid using a path at all when it is not needed ({{this.propName}} is slightly more work than {{propName}}). Also removed unsupported isGlobal check from normalizeClasses.

rwjblue added a commit that referenced this pull request Sep 10, 2015
Ensure attributeBindings work when legacy view addon is disabled.
@rwjblue rwjblue merged commit c665584 into emberjs:master Sep 10, 2015
@rwjblue rwjblue deleted the fix-attribute-bindings branch September 10, 2015 17:44
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.

[canary] Link component no longer binds href.
2 participants