Skip to content

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

Merged
rwjblue merged 2 commits into
emberjs:masterfrom
rwjblue:fix-attribute-bindings
Sep 10, 2015
Merged

Ensure attributeBindings work when legacy view addon is disabled.#12318
rwjblue merged 2 commits into
emberjs:masterfrom
rwjblue:fix-attribute-bindings

Conversation

@rwjblue
Copy link
Copy Markdown
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
Copy Markdown
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 rwjblue force-pushed the fix-attribute-bindings branch from 30439cf to b60e201 Compare September 10, 2015 13:29
@rwjblue
Copy link
Copy Markdown
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