#45711 closed defect (bug) (fixed)
Page listing is no longer hierarchical
| Reported by: | david.binda | Owned by: | desrosj |
|---|---|---|---|
| Priority: | normal | Milestone: | 5.0.2 |
| Component: | Posts, Post Types | Version: | 5.1 |
| Severity: | normal | Keywords: | has-patch |
| Cc: | Focuses: | administration |
Description (last modified by )
With [44185] being in place, the listing of pages on wp-admin/edit.php?post_type=page is no longer hierarchical, because the !isset($orderby) part of the condition in https://core.trac.wordpress.org/browser/branches/5.0/src/wp-admin/includes/post.php?rev=44185#L1089 is no longer true.
Attachments (1)
Change History (12)
This ticket was mentioned in Slack in #core by pento. View the logs.
8 years ago
#2
@
8 years ago
- Component General → Posts, Post Types
- Focuses administration added
- Milestone Awaiting Review → 5.0.2
#5
@
8 years ago
- Keywords has-patch added
Thanks for reporting this, @david.binda!
I think changing this to empty() is OK. There does not seem to be any specific reasoning for using ! isset() over empty()`. I traced this down to [15491].
@TimothyBlynJacobs I plan to open a ticket that documents all uses of compact() in core in order explore removing it (similar to how extract() was removed from core). I am hoping we can explore and establish a better pattern then.
#6
@
8 years ago
- Keywords dev-feedback added
Went through the other compact() related changes to make sure no other similar situations are happening.
#7
@
8 years ago
- Keywords commit added; dev-feedback removed
- Owner set to
- Status new → assigned
Given that WP_Query does an empty() test on the orderby parameter (ref), I don't think there's a practical difference between the orderby parameter not being set, and it being set to an empty value.
Core doesn't call wp_edit_posts_query() with the orderby parameter set to an empty value, so there's no difference to core cause by 45711.diff.
I've had a quick look through the plugins directory, I haven't found any wp_edit_posts_query() calls that would be affected by this change.
45711.diff is 👍🏻 to commit.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Instead of making the default values empty strings, what about making them
null. That seems closer to what the expected behavior would be, short of conditionally building thecompactlist of variables.