Make WordPress Core

Opened 16 years ago

Closed 16 years ago

#2516 closed defect (bug) (fixed)

Page order messed up with latest CVS

Reported by: abbaanthony's profile abbaanthony Owned by:
Milestone: 2.1 Priority: low
Severity: major Version: 2.0.1
Component: General Keywords: Page order child
Focuses: Cc:

Description

I have found that the page order is messed up with the latest CVS. Something in the last day or so has caused children pages to no longer set off. The first child page will indent but the rest don't.

I know it is a very trivial thing but thought I would mention it none the less.

Attachments (1)

patch.2.diff (1.0 KB) - added by markjaquith 16 years ago.
work... please

Download all attachments as: .zip

Change History (14)

#1 @abbaanthony
16 years ago

Forgot to include link so you can see what I mean... http://www.taac.us/wordpress/

It is this way in all the different templates I have so I assume that it is not a template problem and the fact that it was fine until the last day or so, until I got the latest CVS.

#2 @markjaquith
16 years ago

A whole bunch of commits on that functionality have gone into trunk in the last few days.

What seems to be happening is that the </ul> is closing after the first item, instead of after the last item in the subtree. I'm checking into it now.

#3 @ryan
16 years ago

  • Milestone changed from 2.0.2 to 2.1

#4 @ryan
16 years ago

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

(In [3578]) Fix page walk bug. fixes #2516

#5 @markjaquith
16 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Ha, funny. I got exactly that far. Unfortunately, there's still a bug. See http://txfx.net/wp2/

It ascends too much. "Child Page" should be under "Parent Page" ... but it's moved up to the top level.

#6 @markjaquith
16 years ago

Sorry, not "Child Page" ... but "Child2"

#7 @ryan
16 years ago

See what I get for trying to be all fancy and reusable. :-) I'll look at it tomorrow.

#8 @markjaquith
16 years ago

No no, this is good stuff!

I've narrowed it down to a line, but I'm not quite sure what to change it to.

{{{ if ( $page->post_parent == $parent->ID ) {

break;

}

}}}

This needs to be called, and it needs to break, but I'm not sure what test to do.

#9 @markjaquith
16 years ago

Hm, it seems that the $parent we're looking for isn't even in $parents (print_r($parents) before the first array_shift, and $page->post_parent isn't in the $parents array). I'm gonna keep at this, so you can sign off if you haven't already. ;-)

#10 @markjaquith
16 years ago

Ok, I think I got it.

Two problems, you were adding $page to $parents... I changed it to add $previous_page to $parents... Insert "Luke, I am my father" joke here.

Other problem was that you were using array_shift(), which was looking at the "uppermost" parent first, instead of the lowermost. So it was looking at grandparent, not getting a match, and doing the </ul></li> dance. We want to look at parents first and then move up to grandparents only if we don't get a match, so my patch uses array_pop()

I can't believe I'm actually understanding this code... it was absolute soup to me an hour ago. :-D

@markjaquith
16 years ago

work... please

#11 @markjaquith
16 years ago

  • Severity changed from trivial to major

Okay, forget that second part about array_pop()... that's wrong... you were adding via array_unshift() so array_pop() is correct. The problem was just in the break; conditional that you were checking against $parent->ID instead of $parents[0]->ID ... we have to check the next parent up. It was breaking anytime you went up a level from a level > 2 (i.e. 3=>2 or 4=>3). The first parent it checked would be a sibling (expected) so it goes up a level. But then it goes up another level because instead of checking the next parent in line, it checks the current parent, and so the loop runs again.

http://txfx.net/wp2/ has a fairly comprehensive hierarchy, with 4=>3, 4=>2, 4=>1, 3=>2, and 2=>1 transition states.

#12 @markjaquith
16 years ago

Correction (why don't I have the capability of editing comments?):

you were adding via array_unshift() so array_shift() is correct.

#13 @ryan
16 years ago

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

(In [3579]) walk_page_tree() fix from Mark J. fixes #2516

Note: See TracTickets for help on using tickets.