WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#4928 closed defect (bug) (invalid)

$locale var may be set but false

Reported by: ashnur Owned by:
Milestone: Priority: normal
Severity: minor Version: 2.3
Component: I18N Keywords:
Focuses: Cc:

Description

in wp-includes/l10n.php line nr 5.

if (isset($locale))

should be

if (isset($locale) && $locale)

Change History (13)

#1 follow-up: @Viper007Bond
8 years ago

  • Version set to 2.3

Let's not double test. If isset() is in fact not doing the job, let's use something else instead.

http://www.php.net/manual/en/types.comparisons.php

#2 in reply to: ↑ 1 ; follow-up: @ashnur
8 years ago

Replying to Viper007Bond:

it's not doubletesting, it's testing if it's false. i know that in the wordpress source the format @$locale would be normal, but i, myself like this much cleaner method where first i check if a variable is set, then i check it if it's false. hiding errors .. umm. not even notices it's the way of the lazy programmers. you can't avoid the isset check.

#3 follow-up: @Nazgul
8 years ago

I'd go for a if (isset($locale) && !empty($locale)) myself.

#4 in reply to: ↑ 2 ; follow-up: @Viper007Bond
8 years ago

Replying to ashnur:

Replying to Viper007Bond:

it's not doubletesting, it's testing if it's false.

i, myself like this much cleaner method where first i check if a variable is set, then i check it if it's false.

Yes, but again, why test twice when you can just test once?

if ( !empty($locale) ) { ...

Would that not get the job done?

#5 in reply to: ↑ 4 ; follow-up: @Nazgul
8 years ago

Would that not get the job done?

No, because that would throw a warning if the variable is undefined afaik.

#6 in reply to: ↑ 3 @ashnur
8 years ago

Replying to Nazgul:

I'd go for a if (isset($locale) && !empty($locale)) myself.

see, i dunno if it's ok. i mean, that's exactly what I did, so it should, but maybe i was wrong about it and it has to be if (isset($locale) && $locale !== FALSE )). i doublechecked the source, but got confused. maybe someone else should check it too, and see if I'm right. it's not about religion .. how to check if it's empty or not, it's about a really annoying bug with wordpress MU. the !== false version works for me, but that's just me.

#7 in reply to: ↑ 5 @Viper007Bond
8 years ago

Replying to Nazgul:

Would that not get the job done?

No, because that would throw a warning if the variable is undefined afaik.

It wouldn't. if ( $var ) does, if ( !empty($var) ) is the exact same thing, but with no warning.

#8 @Viper007Bond
8 years ago

Oh, and for the record, doing if ( $unsetvar ) { will generate a notice. ;)

#9 @Otto42
8 years ago

What's the actual bug here? What should it be testing for?

If looks to me like you really want to check if $locale is a string, so would is_string() work?

#10 @Viper007Bond
8 years ago

Yeah, I guess is a string and not blank aka empty.

#11 @Otto42
8 years ago

No, the empty string appears to be a valid option here. Because lower down, it explicitly sets $locale to if it's empty, and then applies the same filter.

#12 @ryan
8 years ago

  • Resolution set to invalid
  • Status changed from new to closed

#13 @lloydbudd
8 years ago

  • Milestone 2.5 deleted
Note: See TracTickets for help on using tickets.