#11316 closed enhancement (fixed)
Consider looking through valHooks by element type first, then by nodeName instead of the other way around
Reported by: | mathias | Owned by: | mathias |
---|---|---|---|
Priority: | low | Milestone: | 1.7.2 |
Component: | attributes | Version: | 1.7.1 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description (last modified by )
jQuery#val
contains this line of code: http://jsapi.info/jquery/1.7.1/val#L2317
hooks = jQuery.valHooks[ elem.nodeName.toLowerCase() ] || jQuery.valHooks[ elem.type ];
This is later repeated on line 2361: http://jsapi.info/jquery/1.7.1/val#L2361.
How about switching these around, making it:
hooks = jQuery.valHooks[ elem.type ] || jQuery.valHooks[ elem.nodeName.toLowerCase() ];
This would allow plugin authors to set e.g. jQuery.valHooks.input
without affecting the valHooks for checkbox
etc. Here’s a more detailed use case: https://github.com/mathiasbynens/jquery-placeholder/issues/52
Note that there’s only one item in jQuery.valHooks
that’s based on the node name: jQuery.valHooks.select
. All others are element types.
Change History (9)
comment:1 Changed 12 years ago by
comment:2 follow-up: 4 Changed 12 years ago by
Component: | unfiled → attributes |
---|---|
Milestone: | None → 1.7.2 |
Priority: | undecided → low |
Status: | new → open |
Type: | feature → enhancement |
I agree with this change.
As for your comment, the variable value
would not be appropriate, but the string "value"
doesn't need to be passed there either. It is there for convenience (a reference to the property name being retrieved), but is not used in any of the internal hooks. I think I did that right after writing attrHooks, which always passes the attribute name. We could remove it.
comment:3 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:4 Changed 12 years ago by
Replying to timmywil:
I agree with this change.
Glad to hear! (Also very happy that you set milestone to 1.7.2 — yay!)
As for your comment, the variable
value
would not be appropriate, but the string"value"
doesn't need to be passed there either. It is there for convenience (a reference to the property name being retrieved), but is not used in any of the internal hooks. I think I did that right after writing attrHooks, which always passes the attribute name. We could remove it.
Oh, I see! The second argument confused me and made me think of set
instead. I was just wondering :)
comment:5 Changed 12 years ago by
As discussed on IRC, it looks like jQuery.valHooks.button
’s get
and set
take two and three arguments, respectively. For that reason it’s probably best to just leave the "value"
stuff in there.
Pull request: https://github.com/jquery/jquery/pull/678
comment:6 Changed 12 years ago by
Should we consider this a breaking change? If so it should be deferred to 1.8. I honestly doubt there are a lot of external hooks but people are always surprising us.
comment:7 Changed 12 years ago by
Milestone: | 1.7.2 → 1.8 |
---|---|
Owner: | set to mathias |
Status: | open → assigned |
I highly doubt it will cause a raucous, but I suppose since we're in beta, we should push to 1.8. Thanks Dave.
comment:8 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Fix #11316. Find valHooks first by element type, then by nodeName.
Reverses the previous search order.
Changeset: 2803a5e6f2704691fdd8ce4d34fe961d0192a0fb
comment:9 Changed 12 years ago by
Milestone: | 1.8 → 1.7.2 |
---|
Related question: why is
"value"
(the string literal) used here instead ofvalue
(the variable name)? http://jsapi.info/jquery/git/val#L2372