After updating from 7.x-3.0 to 7.x-3.4 I noticed that the field required marker (*) gets printed in forms even when the field label is not. I attached two images (before/after) of the issue (the form in the images is coming from Views).

It looks like the algorithm used to determine when the label must be shown has changed from the previous version in the bootstrap_form_element_label() function. Basically, the required marker should be appended to the $title string only if the string is not empty, so the function could return properly.

I'm attaching a patch for the 7.x-3.x branch.

CommentFileSizeAuthor
#2 2638300_required_marker_D7.patch1.29 KBproteo
After.png8.48 KBproteo
Before.png8.37 KBproteo

Comments

Proteo created an issue. See original summary.

proteo’s picture

Status: Active » Needs review
StatusFileSize
new1.29 KB

  • markcarver committed b4e9a0c on 7.x-3.x authored by Proteo
    Issue #2638300 by Proteo: "Required" marker is printed even when label...
markhalliwell’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

proteo’s picture

Status: Closed (fixed) » Active

Sorry to open this again. The provided patch is working as intended, but I just noticed a small, related bug, which I think could be addressed in the same patch. It's a very small detail, but we're looking for perfection, aren't we?

Today it was brought to my attention that in some cases, bootstrap_form_element_label() is generating empty <label> elements. It's not very noticeable, only in some edge cases. The problem lies in the line #40:

$title = isset($element['#title']) ? filter_xss_admin($element['#title']) . ' ' : '';

The problem is that isset() can return true even if $element['#title'] is an empty string. In such cases, the line above returns the $title variable as a string containing an space (" "), which is appended after filter_xss_admin() in order to visually separate the required marker. In such cases, even if no marker is appended, the evaluation in line #51, which tries to catch empty titles and return, will fail and an empty <label> element will be generated.

This is very hard to notice because empty elements usually won't alter the HTML rendering. We noticed today because Webkit browsers seem to be more picky and the combination of this and some unusual CSS in surrounding elements generated additional space which broke the layout.

Suggested fix:

$title = isset($element['#title']) ? filter_xss_admin($element['#title']) : '';
  if ($title && ($required = !empty($element['#required']) ? theme('form_required_marker', array('element' => $element)) : '')) {
    $title .= ' ' . $required;
  }
markhalliwell’s picture

Status: Active » Needs work

Actually, I think it would be better to just add $title to the beginning of the condition for $required:

  $title = !empty($element['#title']) ? filter_xss_admin($element['#title']) . ' ' : '';

  // Only show required marker if there is a title.
  $required = $title && !empty($element['#required']) ? theme('form_required_marker', array('element' => $element)) : '';
  if ($required) {
    $title .= $required;
  }
proteo’s picture

Status: Needs work » Reviewed & tested by the community

Sure, that will make it. Just tested on our devel box.

markhalliwell’s picture

Status: Reviewed & tested by the community » Needs work

No actual patch for #7.

  • markcarver committed 0198218 on 7.x-3.x
    Issue #2638300 by Proteo, markcarver: "Required" marker is printed even...

  • markcarver committed fc14976 on 8.x-3.x
    Issue #2638300 by Proteo, markcarver: "Required" marker is printed even...
markhalliwell’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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