button creation that requires a matching label does not find label in all cases

[ddstreet]

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)

[ddstreet]

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.

[Scott González]

I really don't think we should handle this. The problem in every one of these cases is that there's no common ancestor.

[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?

[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.

[ddstreet]

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)?

[Scott González]

Shorter patch: ​https://gist.github.com/864880

[ddstreet]

Possible test case patch ​https://github.com/ddstreet/jquery-ui/commit/893c54adf59ff3b13740e4a263fb5ba30f2953aa

[ddstreet]

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

[ddstreet]

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