[wp-trac] [WordPress Trac] #48312: PHP4 notation for passing object by reference broken

[WordPress Trac]

#48312: PHP4 notation for passing object by reference broken
--------------------------+-----------------------------
 Reporter:  david.binda   |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Future Release
Component:  Plugins       |     Version:  trunk
 Severity:  normal        |  Resolution:
 Keywords:                |     Focuses:
--------------------------+-----------------------------

Comment (by jrf):

 @SergeyBiryukov @davidbinda Thanks for pinging me.

 > the array wrapping is no longer needed, but in case there is a code
 using the old notation (see above), the args are not really being passed
 to the callback in an expected way, eventually producing issues in the
 callbacks.

 I do see the problem and you're correct that it's the array wrapping not
 being removed which is causing it.

 I really would not recommend reverting the patch as it removed a lot of
 redundancy.

 I'm attaching a patch which fixes this specific use-case in a simple way
 by just bringing that one tiny bit back.

 The patch also updates the unit test kindly provided by @davidbinda:
 1. The point of the removed code was to support objects passed by
 reference and the unit test was not passing the object by reference.
 2. The unit test was using short arrays which are a) not allowed CS-wise
 and b) PHP 5.4+ code which would in a way invalid the test.
 3. The point of the passing by reference is that the object passing to the
 functions attached to the action is the same object and not a copy (PHP4),
 so using `assertSame()` actually tests that, while the previously used
 `assertEquals()` did not.

 > Let's do that, this seems like an edge case that could be fixed in a
 minor release if necessary.

 @SergeyBiryukov I'll leave it up to your judgement whether to commit this
 patch now or later.

 I do think this patch is safe and that @davidbinda has a valid point about
 the changed behaviour.

 I've got a build running now to make sure that all tests pass: https
 ://travis-ci.org/jrfnl/wordpress-develop/builds/598323636

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/48312#comment:7>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform