WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#14135 closed defect (bug) (fixed)

edit_posts_per_page

Reported by: tslice Owned by: sorich87
Milestone: 3.1 Priority: normal
Severity: minor Version: 3.0
Component: Plugins Keywords: has-patch regression
Focuses: Cc:

Description

Looks like a filter name was changed for edit_posts_per_page. A workaround has been to hook into both: edit_posts_per_page && edit_post_per_page (note the missing or added 's').

Attachments (8)

14135.diff (896 bytes) - added by wpmuguru 7 years ago.
14135.2.diff (936 bytes) - added by wpmuguru 7 years ago.
14135.3.diff (1.6 KB) - added by wpmuguru 7 years ago.
14135.3.0.1.diff (1.6 KB) - added by wpmuguru 7 years ago.
14135.4.diff (654 bytes) - added by sorich87 7 years ago.
14135.5.diff (795 bytes) - added by sorich87 7 years ago.
Refreshed patch, with edit_pages_per_page
14135.6.diff (681 bytes) - added by sorich87 7 years ago.
Fixes consistency of 'edit_posts_per_page'
14135.7.diff (1.6 KB) - added by sorich87 7 years ago.
Removes 'edit_posts_per_page'

Download all attachments as: .zip

Change History (38)

#1 @wpmuguru
7 years ago

  • Severity changed from major to minor

The reporter is referring to the @todo on line 51 of wp-admin/edit.php. Around line 900 of wp-admin/includes/post.php is the edit_ . $post_type . _per_page.

#2 @nacin
7 years ago

  • Milestone changed from Awaiting Review to 3.0.1

@wpmuguru
7 years ago

#3 @wpmuguru
7 years ago

The patch also fixes #14342. The affected code was all added in 3.0, so both should be fine under 3.0.1.

@wpmuguru
7 years ago

#4 @wpmuguru
7 years ago

The second patch uses the 20 in [15444] and less confusing variable names.

Anyone using these filters would have been using both filters to get the correct display, so switching to using the same filter should not adversely affect anyone.

#5 @wpmuguru
7 years ago

  • Keywords has-patch added; filter hook removed

#6 @nacin
7 years ago

I don't see why the filter needs to be removed? That's a generic filter and should remain if only for back compat. Possibly even added into template.php I'd think.

#7 @wpmuguru
7 years ago

It was added in 3.0, so it's only covering backward compatibility for 33 days. Anyone using it will also have to use the other filter as well or the page count is off.

Do you want me to redo the patch & put the generic one back and add it to includes/post.php?

#8 @nacin
7 years ago

http://core.trac.wordpress.org/browser/branches/2.9/wp-admin/includes/post.php#L853

It used to be edit_posts_per_page. We changed it in wp-admin/includes, but didn't change this instance. Really, both spots should have both filters, generic (old) followed by specific (new). Unless I'm missing something -- does that make sense?

@wpmuguru
7 years ago

#9 @wpmuguru
7 years ago

Added. I also added the post_type to the generic filter.

Even without your explanation above, in the 3.0 cycle we added quite a few of the generic filters because they are useful. So, the generic one should remain here for consistency.

#10 @nacin
7 years ago

  • Keywords commit added

Looks solid.

#11 @nacin
7 years ago

Out of curiosity, anyone else have a preference for which filter comes first?

#12 @wpmuguru
7 years ago

(Now that I looked...) In _option & _transient functions they are the opposite order- specific first, generic second. We should go for consistency.

@wpmuguru
7 years ago

#13 @wpmuguru
7 years ago

I swapped those lines and found I had used the wrong var in includes/post.php.

#14 @ryan
7 years ago

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

(In [15450]) Make posts per page filters consistent and back compat. Props wpmuguru. fixes #14135 for 3.1

#15 @ryan
7 years ago

(In [15451]) Make posts per page filters consistent and back compat. Props wpmuguru. fixes #14135 for 3.0.1

#16 @nacin
7 years ago

We forgot about edit_pages_per_page. Do we care?

#17 @nacin
7 years ago

  • Milestone changed from 3.0.1 to 3.1
  • Resolution fixed deleted
  • Status changed from closed to reopened

#18 @nacin
7 years ago

  • Milestone changed from 3.1 to 3.0.2

@sorich87
7 years ago

#19 @sorich87
7 years ago

The previous changeset did not take into account 'wp-admin/includes/template.php'. The generic filter was not added in this file.

I don't think 'edit_pages_per_page' is necessary, since 'edit_posts_per_page' accepts the post type as parameter.

#20 @sorich87
7 years ago

  • Cc sorich87@… added

#21 @hakre
7 years ago

Replying to sorich87:

I don't think 'edit_pages_per_page' is necessary, since 'edit_posts_per_page'
accepts the post type as parameter.

good point

#22 @nacin
7 years ago

  • Keywords regression added
  • Milestone changed from 3.0.3 to 3.1

That's not the point though, edit_pages_per_page was in 2.9, and now it's not in 3.0.

#23 @nacin
7 years ago

  • Keywords needs-patch added; has-patch commit removed

@sorich87
7 years ago

Refreshed patch, with edit_pages_per_page

#24 @sorich87
7 years ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to sorich87
  • Status changed from reopened to accepted

#25 @ryan
7 years ago

  • Milestone 3.1 deleted
  • Resolution set to wontfix
  • Status changed from accepted to closed

At this late stage, the new way is established. There's little point in adding legacy filters.

#26 @sorich87
7 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Even if we don't want to add the legacy filter 'edit_pages_per_page', the original issue still remains:

The use of the filter 'edit_posts_per_page' is not consistent. That filter should be applied not only to the posts query, but also to the screen options.
Actually, when you use the filter to set the number of posts, the screen option doesn't show your value set via the filter but the default value. There is no consistency between the actual number of posts displayed and the number shown in the screen options.

@sorich87
7 years ago

Fixes consistency of 'edit_posts_per_page'

#27 @nacin
7 years ago

  • Milestone set to 3.1

#28 @ryan
7 years ago

Or remove the two occurrences of edit_posts_per_page.

@sorich87
7 years ago

Removes 'edit_posts_per_page'

#29 @ryan
7 years ago

After IRC discussion, going with 14135.6.diff.

#30 @ryan
7 years ago

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

(In [17082]) Consistently use edit_posts_per_page. Props sorich87. fixes #14135

Note: See TracTickets for help on using tickets.