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

can.observe.delegate fails on compound selectors with wildcards #119

Closed
rjgotten opened this issue Oct 18, 2012 · 2 comments
Closed

can.observe.delegate fails on compound selectors with wildcards #119

rjgotten opened this issue Oct 18, 2012 · 2 comments
Labels
Milestone

Comments

@rjgotten
Copy link

Given a compound selector with a wildcard in it used in an event binding to a can.Observe instance:

ob.delegate( "foo.* bar", "set", function( ob, event, newVal, oldVal, prop ){  })

When any child attribute of "foo" is changed (let's say "foo.baz", for instance), eventually can.observe.delegate will hit line 94, which checks:

valuesEqual = this.attr(attr.attr) !== undefined

As the obseve will never have a property named "foo.*", valuesEqual will be false and the bound event handler will never fire.

Imho this entire test should be removed:

else if (valuesEqual && delegate.attrs.length > 1){
  // if there are multiple attributes, each has to at
  // least have some value
  valuesEqual = this.attr(attr.attr) !== undefined
}

Why is it checking if all attributes have an assigned value when the documentation clearly states that a selector like "foo bar" matches changes to "foo" or "bar" ? If either is valid, the bound handler should fire. Period.

[EDIT]
Hmm.. From the looks of it, the entire value checking logic is busted as well: it assumes 'AND' semantics instead of 'OR' semantics. E.g. for a selector "foo=a bar=b", if "foo" is set to "a", while "bar" is not equal to "b" the selector will not match...

@avhm
Copy link

avhm commented Oct 24, 2012

+1

@imjoshdean
Copy link
Contributor

We will heavily consider this during the next release, but for 1.1 it will stay as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants