WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 15 months ago

Last modified 15 months ago

#12367 closed defect (bug) (invalid)

Fixes for deprecated assignment of new objects by reference

Reported by: technosailor Owned by: westi
Milestone: Priority: normal
Severity: normal Version: 3.2
Component: Warnings/Notices Keywords: has-patch
Focuses: Cc:

Description

SimplePie does a whole lot of instantiation of classes by reference which is now deprecated in PHP 5.3. Though I don't want to do a lot of patching on an upstream library, I think this is an important thing to fix in WP 3.0.

Attachments (3)

simplepie_deprecated_notices.diff (46.0 KB) - added by technosailor 4 years ago.
fix_assign_new_by_reference.diff (50.0 KB) - added by mrtorrent 3 years ago.
Fixes all instances of deprecated assignment by reference of new objects
fix_assign_new_by_reference.2.diff (59.1 KB) - added by mrtorrent 3 years ago.
Updated: Fixes all instances of deprecated assignment by reference of new objects

Download all attachments as: .zip

Change History (22)

comment:1 nacin4 years ago

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

These are necessary for PHP 4 compatibility, as objects in PHP 4 are copied unless passed by reference. We uses this throughout core and deliberately suppress E_DEPRECATED (even when WP_DEBUG).

comment:2 nacin4 years ago

  • Milestone 3.0 deleted

comment:3 scottconnerly3 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

WP 3.2 now requires PHP 5. It would be appropriate to upgrade simplepie to their master branch now.

comment:4 scottconnerly3 years ago

  • Cc scottconnerly added

comment:5 SergeyBiryukov3 years ago

  • Milestone set to Awaiting Review

comment:6 rmccue3 years ago

1.3-dev (the master branch) is unstable, so I wouldn't update to it yet. It's very unlikely that there will be more bugs, but the library has been somewhat reorganised, so it's possible that more bugs will have been introduced.

mrtorrent3 years ago

Fixes all instances of deprecated assignment by reference of new objects

comment:7 mrtorrent3 years ago

  • Cc mrtorrent@… added
  • Keywords needs-testing removed

Hi everyone, I've attached a patch against current trunk to correct all instances of new objects being assigned by reference.

mrtorrent3 years ago

Updated: Fixes all instances of deprecated assignment by reference of new objects

comment:8 mrtorrent3 years ago

  • Summary changed from SimplePie E_DEPRECATED fixes to Fixes for deprecated assignment of new objects by reference
  • Version set to 3.3

Apologies, first patch was incomplete. Corrected patch attached.

Changed issue summary as this now addresses all instances of assignment of new objects by reference and therefore fixes the remaining E_DEPRECATED warnings.

Can this make it into 3.3?

comment:9 rmccue3 years ago

From my point of view, WordPress having a customised version of SimplePie makes it much harder for me to maintain, so I'd prefer to avoid that.

1.2 would most likely be fine to use, but it doesn't have as much testing as I'd like. In addition, it has quite a few backwards incompatibilities. If you'd like to roll that into 3.3, I'll see if I can get some more tests in the mean time.

comment:10 mrtorrent3 years ago

Wordpress already uses Simplepie 1.2. How do these fixes make things harder to maintain?

From my point of view, deprecated PHP4 code like this makes it harder to maintain modern code because it interferes with tests and obviously may break future versions of PHP.

comment:11 rmccue3 years ago

Sorry, I meant 1.3. This is what I get for writing comments before I'm awake. :)

1.3 is the development version, and the first to drop PHP 4 compatibility. It also features major changes (internally, and a few to the API), so I'd like to actually release it before it gets used.

comment:12 follow-up: nacin3 years ago

We don't hack external libraries for things like this, so let's exclude TextDiff and SimplePie from the patch.

Optionally, check to see if TextDiff has been updated upstream.

comment:13 in reply to: ↑ 12 mrtorrent3 years ago

Replying to nacin:

We don't hack external libraries for things like this, so let's exclude TextDiff and SimplePie from the patch.

Optionally, check to see if TextDiff has been updated upstream.

I can understand not wanting to fork functionally from a 3rd-party library so that patches don't have to be maintained, but this is a small compatibility fix that is essentially incorporated in newer versions of both libraries. It fixes a current issue without breaking anything and will not have to be forward-ported if the included versions are updated in the future, so where's the harm?

comment:14 nacin2 years ago

  • Version changed from 3.3 to 3.2

Maintaining a forked version of an external library is an extra burden we'd rather not deal with, especially for suppressed notices that have no real effect. It'd be a better use of time to go through with upgrades to newer versions of both libraries.

comment:15 SergeyBiryukov2 years ago

Closed #12709 as a duplicate.

set_magic_quotes_runtime() (used in PclZip and PHPMailer) is also deprecated.

comment:16 ocean902 years ago

Duplicate of the SimplePie part: #20139

comment:18 wonderboymusic15 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

This is even fixed in TextDiff now, (although there are 5 lingering instances of a callable with &$this, but not an issue)

comment:19 helen15 months ago

  • Resolution changed from wontfix to invalid
Note: See TracTickets for help on using tickets.