serendipity_pickKey oddness

[judebert]

The serendipity_pickKey function works differently than I expected. Here's the code, for a quick reminder (same in 1.2 branch and trunk):

Code: Select all

/* Picks a specified key from an array and returns it
 *
 * @access public
 * @param   array   The input array
 * @param   string  The key to search for
 * @param   string  The default value to return when not found
 * @return null
 */
function &serendipity_pickKey(&$array, $key, $default) {
    if (!is_array($array)) {
        return $default;
    }

    foreach($array AS $child) {
        if (is_array($child) && isset($child[$key]) && !empty($child[$key])) {
            return $child[$key];
        }
    }

    return $default;
}

I was expecting a key from the passed array to be returned. What actually happens is the first key in an array nested within the passed array is returned. Keys in the passed array are not considered.

I checked, and pickKey is not used in any templates. It and its Smarty wrapper, serendipity_smarty_pickKey, are actually used only in the functions_images.inc.php, for serendipity_parseMediaProperties(). It's used to get and set the DATE, RUN_LENGTH, DPI, COPYRIGHT, TITLE, COMMENT1, and COMMENT2 image fields.

These, in turn, are used in the media selector.

I'd like to modify pickKey to work as I expected. Is the media selector displaying the image properties as expected? (I don't have properties on any of my images, so far as I'm aware, so I can't check it.) If it's not, perhaps I've found the problem. If so, I'll need to do something a little more devious.

Anybody use the media metadata in their media selection?

[garvinhicking]

Hi!

I agree, that smarty function is not used in anything official. I also doubt it will be used in any smarty function, as it's not really that useful.

However the pickkey function in the PHP scope does work, and does work for me as intended with media properties...

So changing the smarty function or modified is alright, but please check if you really need to modify the core PHP function and see if the change works with metadata as it (seems to) do now?

Regards,
Garvin

[judebert]

Yup; I found a picture that had EXIF data and tried it out. (With the appropriate config item set.) Sure enough, the existing function expects an array of arrays.

I modified it to check first for keys in the top-level array, then keys in any child arrays. That way it still works for the media manager, but also in the expected manner. I'll be checking it in soon.

[Don Chambers]

OK, Jude's revision beginning on line 1152 of /include/functions.inc.php is as follows:

Code: Select all

// array_key_exists() copies array, so is much slower.
if (in_array($key, array_keys($array))) { 
    return $array[$key];
}

Problem with this piece of code is that pickKey does not work when quicksearch has been called......

If I comment out Judebert's modifications, it works just fine.

I know Judebert is buried with job deadlines, so anyone else have any idea why??

[Don Chambers]

I'm still stumped by this one...... Any thoughts Garvin???

[garvinhicking]

Hi!

Nope, I couldn't inspect this problem.

Regards,
Garvin

[judebert]

Do you mean you couldn't reproduce it, or you don't have time to check it out?

[garvinhicking]

Hi!

I didn't have the time to setup an environment with which to reproduce it

Regards,
Garvin

[judebert]

I just did some experimentation in my own sandbox. I gave each entry a "Friends" description ("The one with...") and modified entries.tpl to print it just before each entry.

I'll be darned, but I'm seeing exactly the same behavior as Don. On the front page, it works. On the search page, it doesn't. I dumped the variables on both pages, but they look correct to me, at least for the purposes of pickKey.

More as it develops.

[judebert]

Okay, got it. Change those lines (around 1154 of functions.inc.php) to:

Code: Select all

    // array_key_exists() copies array, so is much slower.
    if (in_array($key, array_keys($array))) {
        if (isset($array[$key])) {
            return $array[$key];
        }
    }

I just added a check for isset() before returning the key. I have no idea how the key gets into the array, but not set.

I tested this on my sandbox before telling you about it. It's committed, and will soon make its way into the nightlies. Since it's one of the core plugins, it can't be fetched with SPARTACUS.

[Don Chambers]

Glad you found it Judebert!!! Looks like you only committed to trunk - can you also commit to 1.3 branch?

EDIT: Nevermind - I did it myself since I read elsewhere that Judebert is buried.

[judebert]

Thanks, Don, appreciate the boost!

[judebert]

I think I may have figured out why this happened. It's a tiny little minor thing, but it tripped me up. I'm hoping that documenting it here will help me remember it in the future.

According to the PHP website, any value that you pass by reference gets created. What they're really saying is that any value you attempt to reference gets created.

I'll bet we reference the ep_* field when we run through the Quicksearch page. The property gets created, but has no value.

The short result is: always check the values with empty(), especially if they could have been set by being referred to.