Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($compile): handle boolean attributes in @ bindings #14070

Closed
wants to merge 1 commit into from

Conversation

jannic
Copy link
Contributor

@jannic jannic commented Feb 17, 2016

Commit db5e0ff handles initial values of boolean attributes. The same
change needs to be applied inside the attrs.$observe() call.

(I'm not familiar with the angular.js codebase, so this may not be the right way to fix the issue. What I verified is that this fixes a regression with some production code which worked with angular.js 1.2.28 and 1.3.20 but failed with 1.4.7.
So feel free to replace this PR with some better fix, as you like.)

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

1 similar comment
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@gkalpak
Copy link
Member

gkalpak commented Feb 17, 2016

Good catch 👍 (This has been on my TODO list for a while 😃)
There are still a couple of things to do, before we can merge this:

  1. Sign the CLA.
  2. Add one or more tests, that verifies the fix (and prevents future regressions).

Feel free to ping me when/if you make any changes, so I can take another look 😉

@jannic
Copy link
Contributor Author

jannic commented Apr 5, 2016

@gkalpak just signed the CLA a few minutes ago. A test was already added back in February.

@googlebot
Copy link

CLAs look good, thanks!

1 similar comment
@googlebot
Copy link

CLAs look good, thanks!

@jannic
Copy link
Contributor Author

jannic commented Apr 5, 2016

Rebased to current master and manually resolved conflicts with 874c0fd

Commit db5e0ff handles initial values of boolean attributes. The same
change needs to be applied inside the attrs.$observe() call.
@gkalpak gkalpak closed this in 1cb8d52 Apr 9, 2016
gkalpak pushed a commit that referenced this pull request Apr 9, 2016
Commit db5e0ff handles initial values of boolean attributes. The same
change needs to be applied inside the attrs.$observe() call.

Closes #14070
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants