WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

Last modified 6 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)

comment:1 follow-up: Viper007Bond7 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

comment:2 in reply to: ↑ 1 ; follow-up: ashnur7 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.

comment:3 follow-up: Nazgul7 years ago

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

comment:4 in reply to: ↑ 2 ; follow-up: Viper007Bond7 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?

comment:5 in reply to: ↑ 4 ; follow-up: Nazgul7 years ago

Would that not get the job done?

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

comment:6 in reply to: ↑ 3 ashnur7 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.

comment:7 in reply to: ↑ 5 Viper007Bond7 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.

comment:8 Viper007Bond7 years ago

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

comment:9 Otto427 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?

comment:10 Viper007Bond7 years ago

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

comment:11 Otto427 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.

comment:12 ryan6 years ago

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

comment:13 lloydbudd6 years ago

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