#7092 closed bug (fixed)
button creation that requires a matching label does not find label in all cases
Reported by: | ddstreet | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | 1.8.11 |
Component: | ui.button | Version: | 1.8.10 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
When creating a button() for a checkbox or radio box, where an associated label is required, the code does not correctly search for the associated label; it misses several cases as illustrated here: http://jsbin.com/akuqu4/15
Specifically, the current code only finds the label if both the checkbox and label have a common ancestor; it misses cases where:
- the label is a sibling of the checkbox, but neither have a parent
- the label is a uncle of the checkbox (and do not share a common ancestor)
- the label is a cousin of the checkbox (and do not share a common ancestor)
- the label is a nephew of the checkbox (and do not share a common ancestor)
Change History (11)
comment:1 Changed 7 years ago by
comment:2 follow-up: 3 Changed 7 years ago by
I really don't think we should handle this. The problem in every one of these cases is that there's no common ancestor.
comment:3 follow-up: 4 Changed 7 years ago by
Replying to scott.gonzalez:
I really don't think we should handle this. The problem in every one of these cases is that there's no common ancestor.
Yes, exactly, as I said in the description. But IMHO jQuery-ui *should* handle cases where there is not (yet) a common ancestor. The code *already* handles the case where the checkbox and label haven't been added to the DOM yet, so I'm not sure why you're drawing the line between that and this case.
Do you think there should be a core jQuery function for this? Or do you just think nobody should be doing this, so jQuery-ui shouldn't change? Or do you just object to the specific way I handled it in my pull request?
comment:4 follow-up: 5 Changed 7 years ago by
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Replying to ddstreet:
Replying to scott.gonzalez:
I really don't think we should handle this. The problem in every one of these cases is that there's no common ancestor.
Yes, exactly, as I said in the description. But IMHO jQuery-ui *should* handle cases where there is not (yet) a common ancestor. The code *already* handles the case where the checkbox and label haven't been added to the DOM yet, so I'm not sure why you're drawing the line between that and this case.
Do you think there should be a core jQuery function for this? Or do you just think nobody should be doing this, so jQuery-ui shouldn't change? Or do you just object to the specific way I handled it in my pull request?
We intentionally don't handle this, not because no one will ever have this markup, but because if someone does, they know better how to deal with it than we do. The only thing we could do is choose one of several possible combinations of moving one or both elements and wrapping one or both of them. That might seriously break someone's form if they're not expecting it, or they might want it done a different way. We can only choose one, whereas they can choose one of many. So, for this reason we don't feel the fix should be built-in. If such a fix is needed, it needs to be done by the user before calling this widget.
comment:5 Changed 7 years ago by
Replying to rdworth:
Replying to ddstreet:
Replying to scott.gonzalez:
I really don't think we should handle this. The problem in every one of these cases is that there's no common ancestor.
Yes, exactly, as I said in the description. But IMHO jQuery-ui *should* handle cases where there is not (yet) a common ancestor. The code *already* handles the case where the checkbox and label haven't been added to the DOM yet, so I'm not sure why you're drawing the line between that and this case.
Do you think there should be a core jQuery function for this? Or do you just think nobody should be doing this, so jQuery-ui shouldn't change? Or do you just object to the specific way I handled it in my pull request?
We intentionally don't handle this, not because no one will ever have this markup, but because if someone does, they know better how to deal with it than we do. The only thing we could do is choose one of several possible combinations of moving one or both elements and wrapping one or both of them. That might seriously break someone's form if they're not expecting it, or they might want it done a different way. We can only choose one, whereas they can choose one of many. So, for this reason we don't feel the fix should be built-in. If such a fix is needed, it needs to be done by the user before calling this widget.
My pull request worked fine in my tests, so I'm not sure why you are saying you would have to do any strange stuff. Can you at least let me know why you think the patch in my pull request wouldn't work (even though it appeared to for me)?
comment:6 Changed 7 years ago by
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
comment:7 Changed 7 years ago by
Status: | reopened → open |
---|
Shorter patch: https://gist.github.com/864880
comment:8 Changed 7 years ago by
Possible test case patch https://github.com/ddstreet/jquery-ui/commit/893c54adf59ff3b13740e4a263fb5ba30f2953aa
comment:9 Changed 7 years ago by
Resolution: | → fixed |
---|---|
Status: | open → closed |
Button: find associated label even without common ancestor. Fixes #7092 - button creation that requires a matching label does not find label in all cases
Changeset: 0b30a1d450f6b7d529732d6a39c5f4057efaaaf4
comment:10 Changed 7 years ago by
Button: find associated label even without common ancestor. Fixes #7092 - button creation that requires a matching label does not find label in all cases (cherry picked from commit 0b30a1d450f6b7d529732d6a39c5f4057efaaaf4)
Changeset: 2d6ad068733a18e5b69815192e7c0f5614410c56
comment:11 Changed 7 years ago by
Milestone: | 1.9 → 1.8.11 |
---|
Sent a pull request for this: https://github.com/jquery/jquery-ui/pull/147
Ideally, it would be nice if jQuery itself added a function for this, as I'm sure other places in the code may need to do something like this.