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

applying dynamic attribute name has no effect in ie6-7 #333

Merged
merged 6 commits into from
Jul 4, 2012

Conversation

SteveSanderson
Copy link
Contributor

if in binding we have attr: { name: 'cb' + Propery()}
it will not have effect in ie6-7. similar bug #197
was fixed for unique name

@SteveSanderson
Copy link
Contributor

Agreed we fix this (the fix is to use mergeAttributes I guess - I'm happy to do it, as with any of the other fixes)

@SteveSanderson
Copy link
Contributor

Yahuen, could you clarify in what way this isn't already supported? I've added a spec (see above) to show this working, and it already passes on IE6. What aspect of setting the name attribute doesn't work - can you provide sample code?

@mbest
Copy link
Member

mbest commented Jul 3, 2012

Steve, if it already works for attr/name, why would uniqueName need a workaround?

Here's some info about how to reproduce the problem: http://webbugtrack.blogspot.com/2007/10/bug-235-createelement-is-broken-in-ie.html

@mbest
Copy link
Member

mbest commented Jul 3, 2012

I modified the new spec to show how it fails in IE6.

actual value:
"<INPUT data-bind='attr: { name: "newName" }' __ko__1341311374578="ko3">"

should match with pattern:
"name="?newName"?"

@SteveSanderson
Copy link
Contributor

Ah yes, I remember! Thanks for clarifying.

@mbest mbest mentioned this pull request Jul 3, 2012
@mbest
Copy link
Member

mbest commented Jul 3, 2012

This looks good. I've added a small change that fixes #452. Without the fix, the new spec will fail in IE9 with browser mode = IE9 Compatibility View and document mode = IE9 Standards. Ready to merge.

@SteveSanderson SteveSanderson merged commit 2ffcb2c into master Jul 4, 2012
@SteveSanderson
Copy link
Contributor

Thanks! Merged.

At some point we might want to consider moving to feature detection for these kinds of things (to avoid having to know about IE compat modes). There is a tradeoff, though, is it will probably be a fair bit of extra code to do it that way. For now this is great as-is.

@mbest
Copy link
Member

mbest commented Jul 5, 2012

I too was thinking that this would be a good candidate for feature detection. Thanks for taking care of this.

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.

None yet

2 participants