Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core pagination is broken #625

Closed
yellowled opened this issue Aug 15, 2019 · 5 comments · Fixed by #626
Milestone

Comments

@yellowled
Copy link
Member

@yellowled yellowled commented Aug 15, 2019

Back in the day, Bulletproof introduced an extra pagination using page numbers instead of just Previous/Next links. So far, Bulletproof, Timeline and plus9 are the only themes that use it. It is based on the variables $footer_totalPages and $footer_currentPage which come from include/functions_entries.inc.php.. See Timeline for how it's implemented in themes.

I'm not sure when this was introduced – most likely 2.3.0, at least that's when it was reported.

I was able to reproduce it in 2.3.0/PHP 7.3.8 in a ddev stage. Beginning at the index page by clicking the 1 link in the pagination, this points and redirects to /archives/P1.html, but the content is that of page 3, which is also highlighted as the current page in the pagination. Try clicking more pagination links, it basically never points to the expected page/content.

Could be related to stable archives, I guess? I'm not even sure if the core gives us the wrong values or if the calculation in the theme's logic is wrong (maybe due to a Smarty update or something?).

@onli

This comment has been minimized.

Copy link
Member

@onli onli commented Aug 15, 2019

Is it still broken with stable archive off? Because this definitely sounds like a problem with stable archive, the calculations done in the template assume the older ordering.

@yellowled

This comment has been minimized.

Copy link
Member Author

@yellowled yellowled commented Aug 15, 2019

I don't know, I didn't think to test that (and already killed my local test stage).

See https://board.s9y.org/viewtopic.php?p=10452201 (German)

@th-h th-h added this to the 2.3.1 milestone Aug 16, 2019
@th-h

This comment has been minimized.

Copy link
Member

@th-h th-h commented Aug 16, 2019

Could be related to stable archives, I guess? I'm not even sure if the core gives us the wrong values or if the calculation in the theme's logic is wrong (maybe due to a Smarty update or something?).

I would guess that the calculation probably never worked with stable archives - probably nobody used one of the themes with pagination and stable archives at the same time. That will change with stable archives as installation default, as it is now.

Is it still broken with stable archive off? Because this definitely sounds like a problem with stable archive, the calculations done in the template assume the older ordering.

According to the bug reporter, switching stable archives off fixes the problem. Looks like we'll have to change the pagination code in those themes to cope with stable archives when they're on.

th-h added a commit to th-h/Serendipity that referenced this issue Aug 16, 2019
The current page will always be the current page,
regardless of archive sorting order. Page 76 of
86 pages will remain page 76, even if the archive
sorting is changed; it won't become page 10.

Fixes s9y#625 in core.

Themes will have to cope with the sorting order
in their pagination code if they want to display
a descending order for stable archive sorting.

Signed-off-by: Thomas Hochstein <thh@inter.net>
@th-h

This comment has been minimized.

Copy link
Member

@th-h th-h commented Aug 16, 2019

The fix in core is quite easy, I think. The current code will set the current page to (maxPages - currentPage) when stable archives is on, but that's not correct; Page 76 of 86 pages will always remain Page 76, even with stable archives, and not change to page 10 then.

Themes will have the wrong sorting order with stable archives, as they still display pages "1 to x" instead of "x to 1", but that's just a visual thing and needs to be fixed in those themes.

@yellowled

This comment has been minimized.

Copy link
Member Author

@yellowled yellowled commented Aug 23, 2019

Still broken, it seems. Looking at https://board.s9y.org/viewtopic.php?p=10452275#p10452275 (German forum), it seems as if maybe the prev/next links (arrows) that are part of the pagination are now … swapped?

Also, the user in the forum reports that the two paginations (top/bottom of page) in Timeline behave differently, but I guess that might be an issue with the theme rather than with the core.

@th-h th-h closed this in ecd3c9f Aug 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.