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).
Comment | File | Size | Author |
---|---|---|---|
tvi-php54_compat-d7.patch | 2.43 KB | NaX | |
Comments
Comment #1
DuaelFrYour patch seems to alter a lot of code without relation with the notices that you are reporting.
Comment #2
NaX CreditAttribution: NaX commentedAll 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.
Comment #3
Simon Georges CreditAttribution: Simon Georges commentedAren'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?
Comment #4
NaX CreditAttribution: NaX commentedI 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.
Comment #5
kevinquillen CreditAttribution: kevinquillen commentedNot 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).
Comment #6
NaX CreditAttribution: NaX commented@kevinquillen
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.
Ref:
Lots of activity in core relating 5.4
#1498574: [policy, no patch] Require PHP 5.4
Just my 2cents :)
Comment #7
kevinquillen CreditAttribution: kevinquillen commentedI understand that- just checking that there wasn't further effort like shortening code with array dereferencing and the like (definitely breaking < 5.4).
Comment #8
DuaelFrNax, could you try to reroll your patch on last dev version please ?
A few things may have been patched these days.
Comment #9
kevinquillen CreditAttribution: kevinquillen commentedYes, a reroll would be apprecated.
Comment #10
kevinquillen CreditAttribution: kevinquillen commentedPostponing, no reroll supplied and it has been a while.
Comment #11
kevinquillen CreditAttribution: kevinquillen commentedIssue and patch are now outdated. If anyone feels like tackling this, feel free to re-open and post a patch.