Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#21183 closed enhancement (fixed)

Update SimplePie to 1.3

Reported by: rmccue's profile rmccue Owned by: nacin's profile nacin
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: External Libraries Keywords:
Focuses: Cc:

Description

I've just released SimplePie 1.3, which fixes #20999 along with many, many other things.

From previous discussions with nacin, I think WP is going to stick to having SimplePie in one file, so the compiled version is probably what you want.

Change History (16)

#1 @ocean90
13 years ago

  • Cc ocean90 added

#3 @primetimejas
13 years ago

Can anyone help with this? I've gotten the new version partially working but the cache is broken. I've replaced class-simplepie.php with the new 1.3 compiled version which seems to be working fine after updating class-feed.php as follows :

  • lines 6-7

class WP_Feed_Cache {

public static function SimplePie_Cache() {

  • line 17

}

  • lines 59-60

class WP_SimplePie_File {

public static function SimplePie_File() {

  • line 96

}

But I'm getting an error with the cache:
PHP Warning: ./cache is not writeable. Make sure you've set the correct relative or absolute path, and that the location is server-writable.

#4 @nacin
13 years ago

  • Milestone changed from Awaiting Review to 3.5

I think splitting this into multiple files would be very nice. Right now it is too big to even browse in Trac. We would just need to make the current file includes all of the others.

Let's do this now and see what breaks, shall we?

#5 @rmccue
13 years ago

Two options with that: WP could just use the autoloader to do that, or include them all from one master file.

Now that I think of it though, a better option would be a hybrid: include the classes that are almost always used, and let the autoloader handle the rest. That would mean: include SimplePie, SimplePie_Item, SimplePie_Registry, SimplePie_IRI, SimplePie_Content_Type_Sniffer, SimplePie_Decode_HTML_Entities, SimplePie_gzdecode, SimplePie_Parse_Date and SimplePie_XML_Declaration_Parser. The rest are either overriden by WordPress, or are accessor classes that are rarely used, and that case could be handled by the autoloader.

What do you think?

#6 @nacin
13 years ago

There's no guarantee that spl_autoload_register() is available. But, I've been meaning to try this for a while now: if it exists, use it, otherwise, include everything. Hybrid of a hybrid.

#7 @rmccue
13 years ago

Good point, damn SPL and how non-standard it is. Should be easy to detect. There's also the possibility of reimplementing it in userland if it's not defined, which would be nice for plugins, but that's not the topic of this ticket.

I don't have time on my hands to make a patch, unfortunately, but should be pretty easy. Grab the 1.3 release, put the contents of the library/ directory into wp-includes/ (renaming SimplePie.php to class-SimplePie.php or whatever) and write a simple script to include the above classes. There may be one or two I forgot about, but that should cover all the classes used by 90% of people.

#8 follow-up: @nacin
13 years ago

Using a straight autoloader, I found that these classes were loaded using our dashboard and regular RSS widgets:

SimplePie/Misc.php
SimplePie/Cache.php
SimplePie/File.php
SimplePie/Sanitize.php
SimplePie/Registry.php
SimplePie/IRI.php
SimplePie/Locator.php
SimplePie/Content/Type/Sniffer.php
SimplePie/XML/Declaration/Parser.php
SimplePie/Parser.php
SimplePie/Item.php
SimplePie/Parse/Date.php
SimplePie/Author.php

We override both SimplePie_Cache and SimplePie_File. From your list, SimplePie_Decode_HTML_Entities and SimplePie_gzdecode were not used. You did not list SimplePie_Misc, SimplePie_Sanitize, SimplePie_Locator, SimplePie_Parser, and SimplePie_Author, which were.

#9 @nacin
13 years ago

No obvious breakage when updating trunk to 1.3. Memory usage dropped a full MB using the hybrid autoloader.

In the upcoming commit, SimplePie/Core.php is omitted from the non-SPL manual load as trying to include it before we define the class results in a fatal error. No reason to kick it to the end of the file, as I found it was not included in previous WP versions.

#10 @nacin
13 years ago

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

In [21644]:

Update to SimplePie 1.3. props rmccue.

Uses individual files for each class. We now conditionally load only the pieces
we need, resulting in less memory usage. Also easier to maintain now that it is
not a single 387KB file.

fixes #21183.

#11 @nacin
13 years ago

In [21645]:

Use ABSPATH . WPINC rather than dynamically building the include path. see #21183.

#12 @nacin
13 years ago

In [21652]:

Stabilize how WordPress hooks into SimplePie to implement transient caching.

Since a plugin can load a previous (< 1.3) version of SimplePie before we do,
we need to be compatible with our old method of overriding SimplePie_Cache::create().

SimplePie_Cache::create() was converted to static in 1.3 (as it was called),
requiring that we create two different definitions of WP_Feed_Cache (extends
SimplePie_Cache). Instead, we can use 1.3's new object registry, and leave
the old WP_Feed_Cache to SimplePie 1.2 versions.

see #21183.

#13 in reply to: ↑ 8 @rmccue
13 years ago

Replying to nacin:

We override both SimplePie_Cache and SimplePie_File. From your list, SimplePie_Decode_HTML_Entities and SimplePie_gzdecode were not used. You did not list SimplePie_Misc, SimplePie_Sanitize, SimplePie_Locator, SimplePie_Parser, and SimplePie_Author, which were.

Ah, whoops. Decode_HTML_Entities isn't used any more (IIRC, since we use DOMDocument instead now). gzdecode will be used if gzip isn't loaded on the server, which seems to be fairly common from what I've seen, but may be different for WP.

Replying to nacin:

In the upcoming commit, SimplePie/Core.php is omitted from the non-SPL manual load as trying to include it before we define the class results in a fatal error. No reason to kick it to the end of the file, as I found it was not included in previous WP versions.

Yep, that's only for compatibility with 1.3 development versions, so there's no need for it. Great to hear about the memory usage. :)

#14 @SergeyBiryukov
13 years ago

Capitalized file names may increase the number of support questions from users whose FTP clients are configured to convert file names to lower case by default (this crops up on support forums once in a while regarding language files).

Comparing revisions also wouldn't work in that case (wp-includes/Text is capitalized as well), but SimplePie is more prominent.

Just something to keep in mind. I've added a note on the "Installing WordPress" Codex article, perhaps that will be enough.

Last edited 13 years ago by SergeyBiryukov (previous) (diff)

#15 @nacin
12 years ago

  • Type changed from defect (bug) to enhancement

#16 @nacin
12 years ago

In 22599:

Remove SimplePie 1.2/1.3 compatibility code no longer needed with 1.3.1. see #22321. see #21183.

Note: See TracTickets for help on using tickets.