I am current testing PHP 5.4 and found the fixed the following with the attached patch:

Warning: Creating default object from empty value in tvi_load_settings() (line 47 of /sites/all/modules/tvi/includes/tvi.query.inc).
Notice: Undefined property: stdClass::$status in tvi_get_term_info() (line 282 of /sites/all/modules/tvi/tvi.module).
Notice: Undefined property: stdClass::$status in tvi_get_term_info() (line 288 of /sites/all/modules/tvi/tvi.module).
Notice: Undefined property: stdClass::$inherit in tvi_get_term_info() (line 292 of /sites/all/modules/tvi/tvi.module).
Notice: Undefined property: stdClass::$status in tvi_get_term_info() (line 303 of /sites/all/modules/tvi/tvi.module).

tvi-php54_compat-d7.patch2.43 KBNaX
Members fund testing for the Drupal project. Drupal Association Learn more


DuaelFr’s picture

Your patch seems to alter a lot of code without relation with the notices that you are reporting.

NaX’s picture

All the changes are related to the notice. The biggest problem was that tvi_load_settings() would return a object that did not have all the attributes that where being accessed EG: $settings->inherit.

So I removed the ability to optionally return the default object and forced it to always at min return the default object. This way all the object attributes would always be available.

Simon Georges’s picture

Aren't they the same notices already fixed by #1908618: Notice: Undefined property: stdClass::$inherit in tvi_get_term_info()? @NaX, could you try with the -dev version?

NaX’s picture

I did not see that patch. Yes, that patch does address some of the same warnings, but not all of them. I also think that the fix is technically not the best solution. The reason is that IMHO it is considered good practice for a object to always have all its attributes, else it is considered a different object or should make use of getters and setters, hence the removal of the conditional default object. This way the object is always the same regardless. Over time it should reduce mistakes and reduce code duplication because we not having to do extra checks every time we want to access a object variable.

kevinquillen’s picture

Not to sidetrack, is there a reason we're adjusting code for PHP 5.4 compliance even though it isn't required to run Drupal or offered by a lot of hosts yet? I just want to double check that we won't introduce issues for folks on 5.3.x (or even 5.2 - though D7 sites should be 5.3.x).

NaX’s picture

According to the System requirements page the PHP requirements are:

Drupal 7: PHP 5.2.5 or higher (5.3 recommended).
Drupal 8: PHP 5.3.5 or higher.

Our host is currently migrating to PHP 5.4 and PHP 5.3 support will be dropped by the end of the year.

IMHO we should always try to support the latest version of PHP so to prevent major problems when upgrading PHP versions.
We hosts many Drupal sites and the PHP 5.3 upgrade was very painful I would like to avoid as much pain as possible with PHP 5.4.
From PHP 5.2 and 5.3 up it much easier to support the latest version of PHP without breaking compatibility with older versions.

Lots of activity in core relating 5.4
#1498574: [policy, no patch] Require PHP 5.4

Just my 2cents :)

kevinquillen’s picture

I understand that- just checking that there wasn't further effort like shortening code with array dereferencing and the like (definitely breaking < 5.4).

DuaelFr’s picture

Nax, could you try to reroll your patch on last dev version please ?
A few things may have been patched these days.

kevinquillen’s picture

Yes, a reroll would be apprecated.

kevinquillen’s picture

Issue summary: View changes
Status: Needs work » Postponed (maintainer needs more info)

Postponing, no reroll supplied and it has been a while.

kevinquillen’s picture

Status: Postponed (maintainer needs more info) » Closed (outdated)

Issue and patch are now outdated. If anyone feels like tackling this, feel free to re-open and post a patch.