Patch #15 in #131432: #field_prefix and #field_suffix work on textfields only made field prefixes and suffixes available to more form elements, but at the same time moved the field prefix away from begin directly next to the field. Instead, it's *before* the label while it should be directly before the textfield. The attached patch moves the #field_prefix/suffix to the correct location.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

doh! :)

This patch is RTBC - but this entire issue/regression probably means that we need tests :-/

Sivaji_Ganesh_Jojodae’s picture

The patch works as expected but it fails to apply with error message

patching file includes/form.inc
Hunk #1 FAILED at 3017.
1 out of 1 hunk FAILED -- saving rejects to file includes/form.inc.rej

Attached is patch re-rolled from latest build. #762074: Page for editing a shortcut shows the site URL in a weird place and #766562: label element after field-prefix are duplicate of this.

cburschka’s picture

Subscribing to this.

The patch looks ready. I'll see about writing a test later today...

jhedstrom’s picture

Status: Needs review » Needs work
FileSize
25.15 KB
26.04 KB
4.26 KB

I started writing some tests, and when I did, realized I'm not sure of the ideal behavior here. Attached is a patch with some tests, but they aren't passing. Also attached are 2 screenshots. The first one, without the patch, the second with the patch. The patch doesn't appear to be moving the prefix/suffix's at all. Moreover though, in D6 and the way it works now, prefix/suffix have wrapped the entire element, including the label. Changing this seems like it would lead to unexpected behavior.

kkaefer’s picture

This patch is about #field_prefix/suffix, not about #prefix or #suffix. These two properties are distinct. #field_prefix is used for example for placing a value *directly* on the same line of a textfield. #prefix is used to place content before the entire widget. That also means that the tests are not testing for #field_prefix but for #prefix (which didn't change in this patch).

jhedstrom’s picture

Ah, that makes more sense. I'll update the patch tomorrow with valid tests.

jhedstrom’s picture

Status: Needs work » Needs review
FileSize
4.43 KB

Here's the patch with proper tests for #field_prefix and #field_suffix. Also, since I'd already done half the work for #prefix/#suffix tests, I added those as well.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, looks good!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.