This code appears to act differently in PHP 5.2 and 5.3:

    // Get error id
    $error_id = @array_search($error_message, $_SESSION['messages']['error']);

    if ($error_id !== FALSE) {

From https://bugs.php.net/bug.php?id=49657, "array_search returns false in 5.2.x, null in 5.3.0, when $haystack is not an array" - which is presumably why @ is used in front of the function call.

In PHP 5.2 this means that the last inline form error on a page is sometimes lost, I will hopefully post a patch when I figure out something that works on both 5.2 and 5.3!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave’s picture

Status: Active » Needs review
FileSize
559 bytes

This bug is only triggered under very specific circumstances: in my case I have a 'radios' element as the last form element, and when using "Remove all messages", an error on this element is not displayed at all on PHP 5.2 (but works as expected on PHP 5.3).

The actual cause is the difference in return value from array_search() - FALSE instead of NULL - but I think the correct fix is to not recurse into child elements if they are one of the excluded types. The attached patch works for me, as in on PHP 5.2, inline form errors show correctly if a radio button set is the last element with errors on a form.

jamsilver’s picture

FileSize
597 bytes

As pointed out by pontus_nilsson over in #1826242: Notice: Undefined index: #type in ife_element_errors_set() (line 269 of .../ife.module), the above patch introduces a PHP Notice for certain elements which legitimately do not have a '#type' property. I have attached pontus_nilsson's patch here which fixes the problem.

mibfire’s picture

  if (isset($element[$key]) && $element[$key] && (!isset($element[$key]['#type']) || !in_array($element[$key]['#type'], $excluded_fields))) {

should be

  if (isset($element[$key]) && $element[$key] && (!isset($element[$key]['#type']) || !in_array($element[$key]['#type'], $excluded_fields))  && isset($element[$key]['#parents'])) {
MrHaroldA’s picture

Issue summary: View changes
Status: Needs review » Needs work

Setting it back as per mibfire's comment ...

stijndm’s picture

Status: Needs work » Needs review

This might be fixed in the latest dev version. The code has been overhauled. Can you please try out the latest dev?

kenorb’s picture

Status: Needs review » Postponed (maintainer needs more info)
paulvandenburg’s picture

Status: Postponed (maintainer needs more info) » Needs review
FileSize
626 bytes

Rerolled patch and added mentioned change in #3, but cleaned up the if logic.

The changes in dev seem unrelated to this code so I don't think this issue is fixed already.