Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue/5918 - Strict in_array() comparison for Settings API Select Multiple #5919

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

d4mation
Copy link
Contributor

@d4mation d4mation commented Aug 2, 2017

Unless you do a strict comparison for in_array() it can cause unexpected <option>s to be selected once the callback function completes.

For example, if a Setting has the following saved:

$value = array(
    '15-1',
);

And the following is available as $args['options']:

$args['options'] = array(
    15 => 'Should NOT be selected',
    '15-1' => 'Should be selected',
    '15-2' => 'Does not get selected',
)

The<option>s with values 15 AND 15-1 get selected. I suppose this is due to 15 not containing any different characters than 15-1 while 15-2 does contain different characters. According to the PHP Docs, setting it to a Strict Comparison is meant to check against the Type of the Needle but it also forces it to match the entire String it would seem.

#5918

Edit: PHP actually doesn't handle the Array key for the Value that way. The Int is an Int.

Copy link
Contributor

@cklosowski cklosowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually have been down this route before, and we cannot use strict comparisons due to the results you'll see explained here: #3175 (comment)

If we can find a way around that, it'd be nice, but we have to account for some of the keys being of a mixed set of types.

@d4mation
Copy link
Contributor Author

d4mation commented Aug 2, 2017

I see. I saw you mentioned casting the Needles to Strings for comparison purposes but still saving/utilizing as Mixed Types for every other purpose. Is there a particular problem with doing that? Is it simply a performance concern?

@cklosowski
Copy link
Contributor

@d4mation I think the casting change for comparison only might get us past the hurdles there. I think we were already past the point of a major overhaul on the old data. The key is testing with more than just EDD Core. Saving options with another data set that might store the data as an int or a string.

@d4mation
Copy link
Contributor Author

d4mation commented Aug 4, 2017

I looked into casting all the Keys for the stored value to Strings (Which I figured would allow better comparison against the Needle) and it turns out that it isn't possible because PHP is PHP. Tried it myself to confirm and PHP auto-detects that something could be an integer and immediately converts it without asking.

However, casting just the Needle to a String works without needing to use a Strict Comparison for in_array(). This may solve the problem as a String that is equivalent to an Int should still return true, but an actual String like 15-1 should return false for an Int like 15.

@SeanChDavis
Copy link
Contributor

Where does this stand at the moment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants