WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 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 4 years ago.
14135.2.diff (936 bytes) - added by wpmuguru 4 years ago.
14135.3.diff (1.6 KB) - added by wpmuguru 4 years ago.
14135.3.0.1.diff (1.6 KB) - added by wpmuguru 4 years ago.
14135.4.diff (654 bytes) - added by sorich87 4 years ago.
14135.5.diff (795 bytes) - added by sorich87 3 years ago.
Refreshed patch, with edit_pages_per_page
14135.6.diff (681 bytes) - added by sorich87 3 years ago.
Fixes consistency of 'edit_posts_per_page'
14135.7.diff (1.6 KB) - added by sorich87 3 years ago.
Removes 'edit_posts_per_page'

Download all attachments as: .zip

Change History (38)

comment:1 wpmuguru4 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.

comment:2 nacin4 years ago

  • Milestone changed from Awaiting Review to 3.0.1

wpmuguru4 years ago

comment:3 wpmuguru4 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.

wpmuguru4 years ago

comment:4 wpmuguru4 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.

comment:5 wpmuguru4 years ago

  • Keywords has-patch added; filter hook removed

comment:6 nacin4 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.

comment:7 wpmuguru4 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?

comment:8 nacin4 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?

wpmuguru4 years ago

comment:9 wpmuguru4 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.

comment:10 nacin4 years ago

  • Keywords commit added

Looks solid.

comment:11 nacin4 years ago

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

comment:12 wpmuguru4 years ago

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

wpmuguru4 years ago

comment:13 wpmuguru4 years ago

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

comment:14 ryan4 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

comment:15 ryan4 years ago

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

comment:16 nacin4 years ago

We forgot about edit_pages_per_page. Do we care?

comment:17 nacin4 years ago

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

comment:18 nacin4 years ago

  • Milestone changed from 3.1 to 3.0.2

sorich874 years ago

comment:19 sorich874 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.

comment:20 sorich874 years ago

  • Cc sorich87@… added

comment:21 hakre4 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

comment:22 nacin3 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.

comment:23 nacin3 years ago

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

sorich873 years ago

Refreshed patch, with edit_pages_per_page

comment:24 sorich873 years ago

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

comment:25 ryan3 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.

comment:26 sorich873 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.

sorich873 years ago

Fixes consistency of 'edit_posts_per_page'

comment:27 nacin3 years ago

  • Milestone set to 3.1

comment:28 ryan3 years ago

Or remove the two occurrences of edit_posts_per_page.

sorich873 years ago

Removes 'edit_posts_per_page'

comment:29 ryan3 years ago

After IRC discussion, going with 14135.6.diff.

comment:30 ryan3 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.