Make WordPress Core

Opened 15 years ago

Last modified 5 years ago

#9978 assigned enhancement

Sticky Posts are not ordered correctly after selection

Reported by: beaulebens's profile beaulebens Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.8
Component: Query Keywords: has-patch stickies
Focuses: Cc:

Description

In WP_Query, sticky posts are added/moved to the top of the posts array after everything else is selected and ordered according to the query parameters.

Sticky posts are loaded as a list of post_IDs from the database, according to the order they were marked as sticky.

Sticky posts that were not originally part of the returned posts are queried and added into the collection of sticky posts separately.

None of the handling of sticky posts, or the handling of the entire post array after stickies are added, is date ordered by date/title/whatever was requested.

Stickies need to be loaded/extracted into a separate array, ordered (independently) according to the original query, then added to the top of the post array.

Attachments (1)

9978.diff (2.4 KB) - added by wonderboymusic 11 years ago.

Download all attachments as: .zip

Change History (13)

#1 @beaulebens
15 years ago

See Also: #9979 for details on an ordering bug/limitation that will affect the fixing of this bug.

#2 @Denis-de-Bernardy
15 years ago

  • Keywords needs-patch added

#3 @Denis-de-Bernardy
15 years ago

  • Type changed from defect (bug) to enhancement

@wonderboymusic
11 years ago

#4 @wonderboymusic
11 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Future Release to 3.7

Moving to 3.7 for more discussion

Here is what currently happens:

  1. Posts are queried
  2. If the result set contains stickies, those stickies are arbitrarily moved to the top
  3. If there are stickies that weren't queried, and they don't appear in post_not__in, extra stickies are queried and shoved between the stickies at the top and the rest of the post
  4. No "order" is even considered

What 9978.diff does:

  1. Posts are queried
  2. If the result set contains stickies, those stickies are removed from the results
  3. All stickies that aren't in post_not__in are queried in get_posts() with the args order and orderby passed
  4. All stickies are placed at the top after returning ordered
  5. Profit.

Also of note, no stickies are currently added to $this->posts when fields is passed as an arg - expected behavior?

#5 @wonderboymusic
10 years ago

  • Keywords commit added

I still like this one - gonna let it simmer with the commit tag to see if anyone objects

#6 @jeremyfelt
10 years ago

I think we need to figure out how to test this. I just figured out how to duplicate what I think is the issue, but after applying 9978.diff, it got weirder–one sticky post showed up twice. Still working through it now to get a better grasp.

#7 @nacin
10 years ago

  • Keywords commit removed
  • Milestone changed from 3.7 to Future Release

one sticky post showed up twice

#8 @jeremyfelt
10 years ago

// Loop over posts and remove stickies.
foreach ( $this->posts as $i => $post ) {
	if ( in_array( $post->ID, $sticky_posts ) ) {
		// Remove sticky from current position
		array_splice( $this->posts, $i, 1 );
	}
}

In array_splice() here, $this->posts is rekeyed each time. As the posts are looped through, it is possible that $i stops matching and results in the removal of an incorrect post. We can add a 4th parameter to array_splice() to maintain the value, and then reindex it on our own after. Not sure if that's the best approach or not.

Also, orderby is not filled in fill_query_vars(), so an undefined index notice may be thrown when $q['orderby'] is referenced for the sticky post query if the query hasn't been filtered.

Last edited 10 years ago by jeremyfelt (previous) (diff)

#9 @wonderboymusic
10 years ago

yeah, we can let it die for now - we should do a sticky reboot in 3.9 or 3.10 or something

#10 @wonderboymusic
10 years ago

  • Keywords stickies added

#11 @ryan
10 years ago

  • Owner ryan deleted
  • Status changed from new to assigned

#12 @boonebgorges
9 years ago

  • Keywords changed from has-patch, stickies to has-patch stickies

Stickies are a nightmare. See #23336. See also https://core.trac.wordpress.org/ticket/27282#comment:9

Allowing the order in which stickies are returned to be affected by 'order'/'orderby' seems to me like a change in expected behavior. The way I've seen stickies used, displaying in the order in which they were stuck seems like it more closely reflects the intent of the sticker, which is to say that they want to show things that they most recently thought were notable.

My inclination would be to leave the current query behavior as-is, and improve the sticky UI so that people can manually choose the order in which they appear.

The issue of pagination counts, and what happens when a post appears in the original results vs when it doesn't, is covered by #27282.

Note: See TracTickets for help on using tickets.