Redirection location inline textfield should not be wrapped in normal form wrappers

Thanks, good suggestion. I haven't looked at how this is generated, but I imagine we could fix this by setting #theme_wrappers to an empty array.

*Hell* theme_form_element_label() filters all label with filter_xss_admin() and removes inputs that have been added to a title.

Yeah I do recall having difficulty with the whole inline fields problem. You've probably already spent a ton of time figuring this out all over again, but this kind of UI pattern is not easy to accomplish in Drupal, which is too bad because it works so well.

Otherwise the last radio will be selected if you click into the redirect_url input box. This bug is only not visible in the other options as the input boxes are only on the last radio item.

We actually have some JS on the page that is doing the work of selecting the right options (in webform-admin.js, the Drupal.webform.setActive function). I think we actually should *not* put the input fields inside the labels because it causes problems with usability. Every time one of the fields gets focus inside a label, the matching radio button gets selected. This causes a problem when tabbing through the elements on the page with the tab key. It makes it so the radio option changes every time the field is tabbed-through, which we don't want.

What's wrong with putting input in label?

Try using the keyboard to tab through the fields on the page. Last I tried this, every time a field received focus within the label, it caused the matching radio button to be selected, even if no value was entered. This made it difficult to select the first radio button and tab through the remaining fields without the intended choice being changed.

The styling being fixed is a good thing. If we can adjust our current JS to *prevent* this undesirable behavior, that would work. That matching JS code needs to be updated (or if things work without it, remove the JS).

I'm unable to reproduce this. I tabbed through the form and it first selects the current selected radio and than I'm able to change the radio. Do you have a repro plan?`

Upon testing, it looks like the problem I described no longer exists. After testing your patch, I tried IE8/10, Safari, Firefox, and Chrome. All of them worked great.

LABEL have the for="" attribute set and if you click on a label this radio get's selected.

Yep, the code we have currently essentially duplicates what labels were supposed to do. It was a work-around when browsers had a troublesome behavior of selecting radio buttons merely by having fields within their labels receive focus.

Overall this looks really good. I'm going to take a hand at rerolling minor adjustments.

Thanks @hass. I'll work on rerolling with this new patch. The changes I had in mind were:

- Keeping the webform-set-active class and updating the JS behavior so that it still selected the current radio button if a field were changed via keyboard. I'll change to using the class appending approach instead of assignment.
- The theme function theme_webform_element_label() is misleadingly named, since I would think that would be the label used for displaying Webform form labels, but in fact it's only used for displaying these inline radio button labels. I want to rename it to theme_webform_inline_radio_label() to match its actual usage. If we want a different theme function for labels displayed in Webform labels, then that should be a separate function (but right now I can't see any reason to do this).

One more thing, we can kill use of "webform-default-value" entirely and use $element['#attributes']['placeholder']. I think browser support is wide enough to make this acceptable at this point, similar to #1305826: Add support for HTML5 placeholder attribute. That's not entirely related to this issue but it's touching on some of the same code, I wouldn't mind killing it while in here.

This feature is currently marked at a 7.x-3.x issue. I don't like changing theme functions mid-cycle like this, and if it's 3.x then we need to backport these changes to 6.x-3.x too. Would you be okay with this being a 7.x-4.x only change?

Here's my reroll, implementing my comments in #20 and #21.

Thanks for looking it all over. I tested it again and I think this is good to go. It surprised me how many places in Webform we use this 3-radio-button approach.

I just have not understood what the effect is with these set active class. How can I repro this for review?

So to reproduce the JS behavior, use the keyboard to tab down to the "Custom URL" (or other textfield) without changing the radio button. Then type a value into the field and tab out of it. The JS behavior will make it so that the radio button that matches the textfield/select list will be checked if a value has been entered. That makes it so keyboard-only users have a similar experience to mouse users, that when they enter a value the corresponding radio button is checked. For mouse users this happens on click, for keyboard users it will now happen on change.

What's the roadmap for 4.x? Currently it's alpha and this sounds like another year or more until final.

Although it's marked alpha I only have it named as such so that I can continue changing the APIs, stability-wise there are fewer overall bugs than 3.x because the architecting has solved several long-standing problems in 3.x (i.e. proper token and conditional support). I wanted to rename "components" to "fields" but my informal surveying of users has indicated that they would only find that more confusing, so I don't think that will come to pass until something like #118984: Field API based rewrite of webform module happens (which would be Webform 5.x).

What I really don't want to have happen is nobody using Webform 4.x on Drupal 7 and ending up in a situation similar to Views 3 for Drupal 6. If the release is too late in the release cycle, nobody ends up using it and the "default" release stays on the previous version. So I'm motivated to make a final release of 4.x sooner rather than later so I don't have to maintain D6/D7 3.x branches in sync any more. It's the back-porting that's the real pain.

I'm strongly for renaming components to fields!

Hah! I'm on the fence about it. "Components" is a terrible name we've had since Drupal 4.4. "Fields" is clearly a better name but it runs into obvious namespace conflicts with Field module and FormAPI. I made a new issue for official discussion: #2037581: [meta] Rename "components" to "fields"(?) Opinions Wanted

Committed #22 to 7.x-4.x. Thanks @hass!

Hm, this patch broke using tokens in any of the "Custom" value fields. For example if I use [submission:values:myfield:nolabel] as the custom value for my "E-mail subject" field, after I save the form and edit the e-mail again, the default value of the field is truncated to :nolabel]. It looks like this has to do with theme_webform_inline_radio_label() using filter_xss(). Something in that function doesn't like using token values as attributes.

To resolve this, I think we can just remove the call to filter_xss() in our label checking. We're using this label in about a dozen places in Webform, but we know every place where this is done and it's never a user-defined value. The default values of the fields displayed inline within the radio button are already individually sanitized. So overall this function call can be removed without harm. This patch removes the filter_xss() call.

Committed to 7.x-4.x.

Heh, from #15:

Try using the keyboard to tab through the fields on the page. Last I tried this, every time a field received focus within the label, it caused the matching radio button to be selected, even if no value was entered. This made it difficult to select the first radio button and tab through the remaining fields without the intended choice being changed.

This is exactly what happened (again) in Firefox: #2045871: Inline textfields select radio button in Firefox, preventing entry of custom e-mail settings

We may need to roll this patch back.

Yeah I think we have something that will work in #2045871: Inline textfields select radio button in Firefox, preventing entry of custom e-mail settings, thanks for joining in the other issue.

a