This issue attempts to correct various inconsistencies between function definitions where parameters are/are not called by reference. The 7.x patches have been committed. Most effort is currently being put into 6.x-2.x.

Examples of the fixes are:

  1. function init() in various views_handler* - The second parameter should not be by reference, remove & (9 fixes)
  2. function pre_render() in views_handler* - The parameter should be by reference, add & (5 fixes)
  3. options_validate(), options_submit(), value_validate(), value_submit(), exposed_validate() in views_handler* - The first parameter should not be by reference, remove & (10 fixes)
  4. options_validate() and options_submit() in views_plugin_row* - The first parameter should be by reference, add & (3 fixes)

Original:
Rolling a patch shortly to fix a bunch of E_STRICT notices using the latest CVS branch.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Status: Active » Needs review
FileSize
8.94 KB

Mostly fixes inconsistencies in function definitions between parent classes and child classes that need to be exactly the same.

Dave Reid’s picture

Um, oops. Forgot to sign out of my test 'normal' account...

dawehner’s picture

Some of them broke some stuff

Dave Reid’s picture

With the patch I'm still seeing:

Strict warning: Declaration of views_handler_filter_boolean_operator::value_validate() should be compatible with that of views_handler_filter::value_validate()
Strict warning: Declaration of views_handler_filter_node_status::operator_form() should be compatible with that of views_handler_filter::operator_form()
Strict warning: Declaration of views_handler_field_node_new_comments::pre_render() should be compatible with that of views_handler_field::pre_render()
Strict warning: Declaration of views_plugin_argument_validate_node::options_submit() should be compatible with that of views_plugin_argument_validate::options_submit()

dawehner’s picture

This one tackles 1) and 2)

The other ones are very special.

dawehner’s picture

Status: Needs review » Fixed

Thanks for the work!

Dave Reid’s picture

Version: 7.x-3.x-dev » 6.x-3.x-dev
Status: Fixed » Patch (to be ported)

These should likely be backported to 6.x-3.x wherever possible as well.

Dave Reid’s picture

So dereine, is the pre_render inconsistency still an issue then? Should we file a separate issue for that one?

dawehner’s picture

Let's open a new issue: here is one #894274: E_STRICT pre_render

Letharion’s picture

Status: Patch (to be ported) » Fixed

Following the line of connected issues, I get the impression that this has been fixed. Closing.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

dawehner’s picture

Status: Closed (fixed) » Patch (to be ported)

No this wasn't ported yet.

mikeytown2’s picture

mikeytown2’s picture

FileSize
11.68 KB

We should standardize the usage of the ampersand. Here is a patch for 6.x-2.x. Will work on one for 6.x.-3.x. I got smart and searched the code base for the inconsistencies in function definitions, rather than waiting for another error to popup.

mikeytown2’s picture

FileSize
12.32 KB

Still missed one due to function init(&$view, &$options) { VS function init(&$view, &$data) {

mikeytown2’s picture

Status: Patch (to be ported) » Needs review
FileSize
8.43 KB

One thing that had to change was

3.x

-  function options_validate($form, &$form_state) { }
+  function options_validate(&$form, &$form_state) { }

2.x

-  function options_validate(&$form, &$form_state) {
+  function options_validate($form, &$form_state) {

Reason has to do with views/handlers/views_handler_sort.inc

/**
 * A special handler to take the place of missing or broken handlers.
 */
class views_handler_sort_broken extends views_handler_sort {
  function ui_name($short = FALSE) {
    return t('Broken/missing handler');
  }

  function ensure_my_table() { /* No table to ensure! */ }
  function query() { /* No query to run */ }
  function options_form(&$form, &$form_state) {
    $form['markup'] = array(
      '#prefix' => '<div class="form-item description">',
      '#value' => t('The handler for this item is broken or missing and cannot be used. If a module provided the handler and was disabled, re-enabling the module may restore it. Otherwise, you should probably delete this item.'),
    );
  }

  /**
   * Determine if the handler is considered 'broken'
   */
  function broken() { return TRUE; }
}
mikeytown2’s picture

Found another one

Strict warning: Declaration of views_handler_filter_user_name::value_submit() should be compatible with that of views_handler_filter_in_operator::value_submit() in views_include_handler()

killua99’s picture

Take a look to this "patch"

sun’s picture

Status: Needs review » Needs work

I tried the patch in #17 for 6.x-3.x, but I'm still getting lots of PHP Strict warnings when running tests.

dawehner’s picture

There are definitive a ton of problems which were caused by d7 only patches/changes in other d7 only patches.

mikeytown2’s picture

@sun
Can you provide more info on what bits of code is throwing the strict warnings in your case?

patricksettle’s picture

FileSize
13.86 KB

Updated patch for d6-2.x. Includes some additional E_STRICT declaration notices. Still need to go through and check to see if these are required in 3.x.

patricksettle’s picture

@sun
Some of the strict warnings may be related to this ticket: http://drupal.org/node/465332
Which was set as "won't fix" by merlinofchaos due to php4 compatibility issues.

apotek’s picture

So it looks like none of these patches are in 6.x-3.x yet.

I'm still getting tons of strict notices. Of course, you can turn these off, but this is my development box, and these kinds of notices are very *germane* (ie, inconsistent declarations are clear pathways for bugs to appear.

Can we say 6.x-3.x should drop PHP 4 support and commit these?

developer_426’s picture

Version: 6.x-3.x-dev » 7.x-3.1

Hi there,

How to use the patch file for avoiding strict warning issues as i am getting such warning while using flag friend with views.

Thanks,

dawehner’s picture

Version: 7.x-3.1 » 6.x-3.x-dev

Let's ignore this scope changing.

@developer_426: For specific issues please create a new issue, with the error :)

patricksettle’s picture

FileSize
13.68 KB

Another updated patch for d6-2.x.
Has anyone gotten a chance to look at 3.x for this?

patricksettle’s picture

FileSize
13.6 KB

Cleaned out some whitespace changes, and added fix for pre_render compatibility for 2.x patch.

monil-dupe’s picture

Thanks hyrcan for this patch.

apotek’s picture

Status: Needs work » Needs review

@hyrcan, thanks for the 6x-2x patch. I changed the status of the issue back to "needs review" so your patch can be tested. If it passes the tests, I'll try to roll one for 6.x-3.x based on it.

Status: Needs review » Needs work

The last submitted patch, views-893128-28-d6-2x.patch, failed testing.

Olafski’s picture

Hi,
thanks for the patch #28. I applied that one plus #11 of #465332: Strict warning: non-static method view::load() should not be called statically, and all warnings went away, except the following one, which was not mentioned yet:

strict warning: Declaration of content_handler_field_multiple::pre_render() should be compatible with views_handler_field::pre_render(&$values) in MY-DRUPAL-SITE/sites/all/modules/cck/includes/views/handlers/content_handler_field_multiple.inc on line 322.

Views version is 6.x-2.16.

EDIT: Sorry, maybe I was too fast. The above mentioned message disappeared after the next page load, I guess that was a cache issue (otherwise I'll keep you informed).

Olafski’s picture

Addendum to #32: Unfortunately I was not too fast: The mentioned warning still appears.

Does anyone know if that message is a serious issue? I'm trying to clarify PHP 5.3 5.4 issues in a test environment before switching the PHP version at the live site.

merlinofchaos’s picture

#32: The error you see is in CCK module, not Views, so you'll want to pursue it there. The error is probably not going to affect functionality, so it is merely annoying.

carillpower’s picture

Status: Needs work » Needs review

#28: views-893128-28-d6-2x.patch queued for re-testing.

inky@inky3d.com’s picture

Is there any chance there will be a patch for 6.3x?

A hosting company that has a number of Drupal6 sites is forcing an upgrade to PHP 5.4 later this year, and views breaks completely when this is enabled. It's not just a matter of error messages, it causes blank sections where a views page/block should be.

dawehner’s picture

Well, I don't have an answer how to solve the php < 5 problem. mhhh

dawehner’s picture

Well, I don't have an answer how to solve the php < 5 problem. mhhh can you please describe what else than the strict notices described in the patch are problematic?

steinmb’s picture

Could we branch create a 4.x branch that not support PHP 4.x?

Letharion’s picture

No one is likely to start a new branch for D6 at this stage. PHP4 goes out soon anyway since D8 is coming out and D6 will loose support.

mattwmc’s picture

Is it possible one fix is still missing?

Updated: And getting these:

Strict warning: Only variables should be passed by reference in uuid_views_data() (line 20 of /home/public_html/modules/uuid/uuid.views.inc).
Strict warning: Only variables should be passed by reference in uuid_views_data() (line 20 of /home/public_html/modules/uuid/uuid.views.inc).
Strict warning: Only variables should be passed by reference in uuid_views_data() (line 20 of /home/public_html/modules/uuid/uuid.views.inc).
Strict warning: Only variables should be passed by reference in uuid_views_data() (line 20 of /home/public_html/modules/uuid/uuid.views.inc).
Strict warning: Only variables should be passed by reference in uuid_views_data() (line 20 of /home/public_html/modules/uuid/uuid.views.inc).
Strict warning: Only variables should be passed by reference in uuid_views_data() (line 20 of /home/public_html/modules/uuid/uuid.views.inc).
mikeytown2’s picture

@mattwmc
I would post an issue in the uuid module

mattwmc’s picture

Thanks for the reply.

Will do and I've been getting others since upgrading to php 5.4 but managed to fix some of them myself.

zionduc’s picture

FileSize
543 bytes

I'v applied the #27 patch (just like they do in OpenAtrium 1.x file: openatrium.make-php-strict) but after this patch I got an extra error message:

strict warning: Declaration of views_handler_filter_user_name::exposed_validate() should be compatible
with views_handler::exposed_validate($form, &$form_state) in
[path-of-the-site]sites/all/modules/views/modules/user/views_handler_filter_user_name.inc on line 6.

So I'v created an other little patch for this issue.
I hope this helps for others.

joelpittet’s picture

@zionduc thanks for that patch, i moved it over to it's own issue because it should be easily fixed without affecting PHP4

#2075561: views_handler_filter_user_name::exposed_validate() form variable shouldn't be a reference variable.

PetarB’s picture

Could someone just post a ZIP of the patched files? I find it difficult to patch anything but the most simplest of amendments, since I have to do it by hand.

mihai7221’s picture

jonathan1055’s picture

Issue summary: View changes
FileSize
1.52 KB

I'm using Views 6-x-2.16 with php5.4 and I applied the patch from #28. This fixed 15 E_STRICT 'declaration of ... should be compatible with ...' warnings - Thank you.

However, in doing so, it introduced 4 new warnings for compatibility with views_handler_field::pre_render(&$values). One of these is fixed by patch #30 in #465332: Strict warning: non-static method view::load() should not be called statically but the other three remain.

Here is a patch with the details. Maybe this should be added to the patch in #28 as it is not a separate issue, just a more comprehensive fix alog with the changes made in that patch.

Jonathan

Francewhoa’s picture

jonathan1055’s picture

Title: Fix E_STRICT notices » Fix E_STRICT notices - byref discrepancies with init(), pre_render(), *_validate() and *_submit()
Version: 6.x-3.x-dev » 6.x-2.x-dev
Issue summary: View changes
Priority: Minor » Normal
FileSize
15.04 KB

Here is the patch from #28 with the four pre_render() warning fixes that I mentioned in #48 added into it. The original patch in #28 changed pre_render($values) to pre_render(&$values) in handlers/views_handler_field.inc. This needs to be replicated for modules/taxonomy/views_handler_field_term_node_tid.inc, modules/upload/views_handler_field_upload_description.inc, modules/upload/views_handler_field_upload_fid.inc and modules/user/views_handler_field_user_roles.inc

From #18 onwards we've only had 6.x-2.x patches, so that is what I am concentrating on.

jonathan1055’s picture

Title: Fix E_STRICT notices - byref discrepancies with init(), pre_render(), *_validate() and *_submit() » Fix E_STRICT notices - byref discrepancies with init(), pre_render(), _validate() and _submit()
Issue summary: View changes

Forgot to say, patch was against latest dev 6.x-2.16+6 dated 2013-10-19, but also applies cleanly to the 6.x-2.16 release.

ibnkhaldun’s picture

I guess: Perhaps a wrong patch direction

I've just read the proposed patch 50. It suggest :
-function ( &$var1, &$var2 )
+function ( $var1, &var2 )
over many methods. As php pass objects by ref, but not arrays() code suggest $var1 is passed by value, but many times $var1 will pass by ref. In particular if $var1 is a std form.
More. As old code expect $var1 passed by ref, new code won't pass arrays by ref... so you wont get a warning but can't do with your arrays wath you expect to do, particularly arrays won't be changed in those methods.

I think, the main way to patch is adding the & where needed instead of removing.

greetings

LgQ

hansfn’s picture

This seems to be a duplicate of #1543434: strict warnings with PHP 5.4.x (which has been committed). Please consider closing this one.

bstrange’s picture

@hansfn,

There are three similar topics and at this point, I'm not sure which one is 'correct'... I tried:

  • the patch from #11 0n (https://www.drupal.org/node/465332#comment-9469777)
  • then (#26) linked patch from (https://www.drupal.org/node/893128#comment-5680104) (#28) and still had 2 warnings & none of my views would render
  • then applied #43 and the increment, and then #52 from (https://www.drupal.org/node/1543434#comment-9472837) and still no joy on my production site! Still getting the following error:
    strict warning: Declaration of content_handler_field_multiple::pre_render() should be compatible with views_handler_field::pre_render(&$values) in Unknown on line 0.
    
    strict warning: Declaration of date_handler_field_multiple::pre_render() should be compatible with content_handler_field_multiple::pre_render($values) in Unknown on line 0.

    I tried to troubleshoot them myself but since they are both from "Unknown on line 0" ; I'm at a bit of a loss.

    Any help would be greatly appreciated!

    BTW: I also tried the Dev release (https://www.drupal.org/node/1543434) but no luck. Nothing rendered with Views loads on my site and at this point I'm thinking maybe between all the various patches, maybe there are a few applied that shouldn't be ... or perhaps I'm missing one... who could tell lol

  • SylvainM’s picture

    Status: Needs review » Needs work
    FileSize
    18.48 KB

    Here is the patch from #50 with another error fixed, but my view still don't work :(

    SylvainM’s picture

    FileSize
    16.79 KB

    Oups, sorry for previous patch, it was mixed with an other issue patch. Here is another one containg juste one more fix

    joachim’s picture

    diff --git a/sites/all/modules/views/handlers/views_handler_argument.inc b/sites/all/modules/views/handlers/views_handler_argument.inc
    

    Patch filenames are incorrect -- roll a patch from the root of the contrib module.

    Also, is this issue covered by the patch that's been committed at #1543434: strict warnings with PHP 5.4.x?

    joachim’s picture

    Title: Fix E_STRICT notices - byref discrepancies with init(), pre_render(), _validate() and _submit() » Fix E_STRICT notices - method declaration compatibility with init(), pre_render(), _validate() and _submit() with PHP 5.4.x
    Status: Needs work » Needs review
    FileSize
    7.75 KB

    It turns out that some of the patch at #56 was already fixed by #1543434: strict warnings with PHP 5.4.x, but quite a few things were not.

    Here is a reroll, with things already fixed removed, and a few more fixes I've added (for declarations of pre_render()).

    pwolanin’s picture

    I'm still seeing this, but last me make sure I have the latest patches applied:

    strict warning: Declaration of views_handler_filter_node_status::operator_form() should be compatible with views_handler_filter::operator_form(&$form, &$form_state) in views/modules/node/views_handler_filter_node_status.inc on line 13.
    
    joachim’s picture

    Status: Needs review » Needs work

    *sigh*

    The trick might be to write a quick few lines of code that load up every class file in Views, so we get all the error messages in one go:

      views_include_handlers();
      $handler_definitions = views_discover_handlers();
      //dsm($handler_definitions);
      foreach ($handler_definitions as $definition) {
        _views_create_handler($definition);
      }
    

    With the above patch, that gets me the following errors:

    strict warning: Declaration of views_handler_filter_term_node_tid_depth::operator_options() should be compatible with views_handler_filter_in_operator::operator_options($which = 'title') in /Users/joachim/Sites/snp-drupal/sites/all/modules/contrib/views/modules/taxonomy/views_handler_filter_term_node_tid_depth.inc on line 89.
    strict warning: Declaration of views_handler_filter_date::exposed_validate() should be compatible with views_handler::exposed_validate(&$form, &$form_state) in /Users/joachim/Sites/snp-drupal/sites/all/modules/contrib/views/handlers/views_handler_filter_date.inc on line 157.
    

    (and some errors from other contrib modules' views handlers too...)

    I'm not seeing operator_form() though, and a search of the code of my version shows the definitions of that all match.

    joachim’s picture

    Status: Needs work » Needs review
    FileSize
    8.95 KB

    Here's an updated patch an interdiff.

    joachim’s picture

    Status: Needs review » Needs work
    joachim’s picture

    Status: Needs work » Needs review

    The last submitted patch, 56: _views-893128-56-d6.2.x.patch, failed testing.

    v8powerage’s picture

    Is patch from #61 gonna fix all errors Joachim?

    joachim’s picture

    I hope so. Try it!

    younggeezer’s picture

    FYI, the change in the args to pre_render propagates complaints into every module with a views interface: image_attach, cck, date...

    joachim’s picture

    There's a patch to CCK, that I know at least.

    v8powerage’s picture

    Hi Unfortunately it didn't do the trick..

    tsphethean’s picture

    Issue tags: +php5.4
    Jorrit’s picture

    Is it an idea to open a separate issue for 6.x-3.x? This will allow the test bot to test the patch on the right branch. Attached a patch with my current changes for this branch.

    fonant’s picture

    #61 works quite nicely here, for Views 6.x-2.18 and PHP 5.5.

    v8powerage’s picture

    I disabled error reporting in htacces as i had no patience for it anymore..

    GreenSkunk’s picture

    Thanks for the patches!

    DamienMcKenna’s picture

    DamienMcKenna’s picture

    Status: Needs review » Reviewed & tested by the community

    Another +1 for the patch in #61.

    • dawehner committed 487d509 on 6.x-2.x authored by joachim
      Issue #893128 by mikeytown2, joachim, hyrcan, dawehner, jonathan1055,...

    • dawehner committed 7dc0b71 on 6.x-3.x authored by joachim
      Issue #893128 by mikeytown2, joachim, hyrcan, dawehner, jonathan1055,...
    dawehner’s picture

    Status: Reviewed & tested by the community » Fixed

    Thanks a ton, committed #61 to both 6.x-2.x and 6.x-3.x

    Feel free to open up additional issues if more things are needed

    jackalope’s picture

    I'm using the most recent views-6.x-2.x-dev and am still getting a ton of these sorts of errors on a PHP 5.4 site:

    Declaration of views_plugin_row::options_submit() should be compatible with views_plugin::options_submit($form, &$form_state) in /var/www/sitename/httpdocs/sites/all/modules/views/includes/handlers.inc on line 76.

    It seems like these should be cleared up by the newest dev version or the patch in #61, but they're not; am I missing something else that needs to be fixed here?

    DamienMcKenna’s picture

    @jackalope: Please open new issues for other things you find, as dawehner requested in comment #82.

    jackalope’s picture

    OK, will do @DamienMcKenna! Wasn't sure if this was a different thing I was finding, or the same thing that this ticket was meant to address.

    jackalope’s picture

    False alarm anyhow: I'd mistakenly applied this other patch to the newest 6.x-2.x-dev, and that is what caused these errors. Without that patch, those errors are now gone--whew!

    Status: Fixed » Closed (fixed)

    Automatically closed - issue fixed for 2 weeks with no activity.

    IFL Todd’s picture

    I applied the patch from #61, and I no longer get the Views module warning; but, still the following views related warnings about Declaration of child method should be compatible with parent method in CCK, Date, Nodequeue, UC_Views, Views_RSS, Views_Attach, and others possibly.

    strict warning: Declaration of content_handler_field_multiple::pre_render() should be compatible with views_handler_field::pre_render(&$values) in  sites/all/modules/cck/includes/views/handlers/content_handler_field_multiple.inc on line 322.
    
    strict warning: Declaration of date_handler_field_multiple::pre_render() should be compatible with content_handler_field_multiple::pre_render($values) in  sites/all/modules/date/date/date_handler_field_multiple.inc on line 185.
    
    strict warning: Declaration of nodequeue_handler_field_all_queues::pre_render() should be compatible with views_handler_field::pre_render(&$values) in  sites/all/modules/nodequeue/includes/views/nodequeue_handler_field_all_queues.inc on line 77.
    
    strict warning: Declaration of uc_views_attribute_handler_field_combination::pre_render() should be compatible with views_handler_field::pre_render(&$values) in  sites/all/modules/uc_views/uc_views_attribute/views/uc_views_attribute_handler_field_combination.inc on line 153.
    
    strict warning: Declaration of views_rss_handler_field_term_node_tid::pre_render() should be compatible with views_handler_field_term_node_tid::pre_render(&$values) in  sites/all/modules/views_rss/views/views_rss_handler_field_term_node_tid.inc on line 171.
    
    strict warning: Declaration of uc_views_handler_field_cart_user::init() should be compatible with views_handler_field::init(&$view, $options) in  sites/all/modules/uc_views/views/uc_views_handler_field_cart_user.inc on line 78.
    
    strict warning: Declaration of views_attach_plugin_display_node_content::options_submit() should be compatible with views_plugin_display::options_submit(&$form, &$form_state) in sites/all/modules/views_attach/views_attach_plugin_display_node_content.inc on line 248.

    Does this need to be an individual issue raised with each module?

    Please advise. Thank you so much,
    Todd

    SeanA’s picture

    "Does this need to be an individual issue raised with each module?"

    Yep! It's usually just a matter of adding or removing an "&" symbol somewhere. Or try to ignore the warnings until they eventually get fixed.