Image sorting broken since 1.5.8

mkanoute

Hello,
I just upgraded from 1.5.7 to 1.5.8 and sorting of images in albums seems broken: next_image() now returns images out of order (I have a custom theme bu it has been working for years, upgraded after upgraded). The expected order is by date descending. It was working until 1.5.7. Could it be related to the use of a context?

Regards,
Mahamane

mkanoute

Trying do debug what is going on: does not seem related to contextes. Album.getImages() does not return the images in the right order.

mkanoute

The problem seems to be in functions.php, sortByKey().
For sorttype=date sortdirection=DESC
Album.sortImageArray() gets the data in the right order from the DB. It turns out that the images name include the date and time, making debuging easier (format=YYMMDD_HHMMSS) =>
{15894:Wed, 23 Jun 2021 04:32:33 GMT}
Album.sortImageArray(): filename=210622_150417_geraniums.jpg
{15894:Wed, 23 Jun 2021 04:32:33 GMT}
Album.sortImageArray(): filename=210607_191609_camelia.jpg
{15894:Wed, 23 Jun 2021 04:32:33 GMT}
Album.sortImageArray(): filename=210607_182848_clematite.jpg
{15894:Wed, 23 Jun 2021 04:32:33 GMT}
Album.sortImageArray(): filename=210607_182727_camelia.jpg
{15894:Wed, 23 Jun 2021 04:32:33 GMT}
Album.sortImageArray(): filename=210607_173926_clematite.jpg
{15894:Wed, 23 Jun 2021 04:32:33 GMT}
The records are storeds in the array $results.

Then $results is sorted by
$results = sortByKey($results, str_replace('`', '', $sortkey), $order);
and then $results is no longer sorted correctly.

mkanoute

Digging further, the problem occurs when calling sortMultiArray() with $natsort = true
Called with false in sortByKey() solves the issue:
$results = sortMultiArray($results, $indicies, $order, true, false, true); -> does not work
$results = sortMultiArray($results, $indicies, $order, false, false, true); -> works

acrylian

Thanks for investigating. The array sroting has indeed been extended to use "real natural" order if the Collator class is available on your system. The oder especially on non English terms may otherwise not be correct without it as plain PHP natsort does not cover it. I am not sure why this would affect sorting by filename actually.

In any case we will try to reproduce it.

But please tell iif your system has the Collator class available (https://www.php.net/manual/de/class.collator.php)? Because the sorting behaviour is different with and without even for "normal" strings. Sorting by date should also work with it but perhaps it needs an exception for date sorting to use the "standard" natsort.

mkanoute

Thanks for looking into this.
Yes, the class Collator exists on my system (I can instantiate it as in new Collator('en_US'); for example)

acrylian

It is really weird, so far we cannot reproduce any issue with sorting by date (the date stored in the database).

You mention filenames like 210622_150417_geraniums.jpg. Are you perhaps actually sorting by filename. Those numbers would not really work as date sorting, it would have to be internation format YYYY-MM-DD to work properly as "strings".

mkanoute

As shown above, sorttype=date sortdirection=DESC. Could it be the descendant direction the issue?

acrylian

It should not, we'll try to reproduce.

acrylian

I am sorry; I tried again and image sorting by date ascending and descending does work for me as intended (with Collator class as well).

Be sure no cache plays tricks on you, e.g. visit the site loggedin and clear your browser cache by force. The latter is very persistent in almost all browsers and a constant source of confusion.

mkanoute

It is not a browser cache issue.

I will keep my code change. Thanks for looking at it.

acrylian

Are you doing something specific somehow?I tried with a standard album image thumb page and here all worked.

J_C

I just added three videos, after updating to 1.5.8 from 1.5.7 the other day, and noticed that sorting by filename is now broken for me after the update. I’m on PHP version 7.4.20.

steam4m

I came here to report the same issue that's appeared after upgrade to 1.5.8 (to solve missing metadata with Multiverse bug!).

Sorting by date is broken across albums that haven't been touched for months (eg https://gallery.steam4me.net/index.php?album=trams/2020) but tried sorting by filename (eg https://gallery.steam4me.net/index.php?album=trams/2021) and that seems to be working ok.

acrylian

@J_C: Example please, especially what filenames you are using. The Collator class does realy natural ordering if available which is a bit different from the standard natural order PHP provides. That was actualyl raelly wrong for especially some languages (e.g. umauts in my native language German).

@steam4m: I can only tell that I cannot reproduce any issue with sorting by date.

In any case we always need the info if your system supports the collator class or not as that is cruical to investigate anything. Review phpinfo if the intl PHP extension is available or re-run setup to see if it is noted as not available.

acrylian

All right, I can reproduce the issue now. What does not work is the setting "Options -> Gallery -> Sort gallery by" while the individual setting on an album does.

Please verify this.

COnfused my self, need more coffee…, of course setting album sorting does not work on images ;-)

Opions -> Image -> Sort images by is the global setting and that also works for me as intended.

Sorry guys…

acrylian

I should point out that all tests so far are done with Collator class support. Neither me nor @fretzl can reproduce your isssues so far.

J_C

Here’s the subalbum I first noticed it in today. I just changed it from sort by Manual back to Filename, when I did this is the order that came up. I will tell you that when it was on sort by Filename earlier, some of the 2021 files were mixed in with the 2020 (the "Name" and "More info" have been changed to obscure the actual filenames, but I don't think that should affect the sort order since they start with dates).

2020-11-14 - Name - More info.mp4
2020-12-31 - Name - More info.mp4
2020-8-9 - Name - More info.mp4
2020-11-19 - Name - More info.mp4
2020-8-16 - Name - More info.mp4
2020-8-13 - Name - More info.mp4
2020-8-13 - Name - More info.mp4
2020-8-13 - Name - More info.mp4
2020-5 - Name - More info.mp4
2020-4-15 - Name - More info.mp4
2020-8-18 - Name - More info.mp4
2020-12-25 - Name - More info.mp4
2020-12-22 - Name - More info.mp4
2020-12-20 - Name - More info.mp4
2020-12-2 - Name - More info.mp4
2020-12-18 - Name - More info.mp4
2020-12-18 - Name - More info.mp4
2020-12-12 - Name - More info.mp4
2020-11-20 - Name - More info.mp4
2020-12-23 - Name - More info.mp4
2021-2-6 - Name - More info.mp4
2021-6-23 - Name - More info.mp4
2021-6-14 - Name - More info.mp4
2021-5-23 - Name - More info.mp4
2021-5-16 - Name - More info.mp4
2021-3-6 - Name - More info.mp4
2021-3-16 - Name - More info.mp4
2021-1-30 - Name - More info.mp4
2021-1-7 - Name - More info.mp4
2021-1-4 - Name - More info.mp4
2021-1-4 - Name - More info.mp4
2021-1-31 - Name - More info.mp4
2021-1-30 - Name - More info.mp4
2021-1-20 - Name - More info.mp4
2021-1-16 - Name - More info.mp4
2021-6-23 - Name - More info.mp4

The Collator class appears to be off when I tested for it in two languages.

PHP info has this for intl-

intl
Internationalization support enabled
ICU version 69.1
ICU Data version 69.1
ICU TZData version 2021a
ICU Unicode version 13.0

Directive Local Value Master Value
intl.default_locale no value no value
intl.error_level 0 0
intl.use_exceptions 0 0

acrylian

If you have the intl class the collator class is included.

The problem you are encountering is that your filename - if I understand corrctly
- have the date as prefix. And the date is not consitently following the yyyy-mm-dd internatlional date format. Like 2020-8-9which should be 2022-08-0. This is likely causing the wrong ordering as the real natural order really sortes by that. As the general order is ascending by year, just the months and dates are mixed up. Dates win other formats are never sorted correctly as they are as "strings".

If you can try with an album with plain alphabetical names. If that works for you as intended this is in your specific case the reason.

Otherwise we would need to add yet another option so users can choose between natural order and real natural order.

acrylian

If you like to help, find the function sortArray() function within zp-core/functions.php. there you find the part:

       if (class_exists('collator')) {
            $locale = getUserLocale();
            $collator = new Collator($locale);
            if ($case_sensitive) {
                $collator->setAttribute(Collator::CASE_FIRST, Collator::UPPER_FIRST);
            }
            $collator->setAttribute(Collator::NUMERIC_COLLATION, Collator::ON);
            $success = $collator->asort($array);
        } else {
            if ($case_sensitive) {
                $success = natsort($array);
            } else {
                $success = natcasesort($array);
            }
        }

Remive the collator if check so it uses always the standard PHP natural order (which often is really wrong in some languages with umlauts, accents and other things):

      if ($case_sensitive) {
                $success = natsort($array);
            } else {
                $success = natcasesort($array);
            }

We really did a lot tests with this sorting and the wrong ordering of in this case French language terms reported by a user was the reason to add this.

acrylian

We might have found something indeed. It might be that while the "real natural order" is way better with real text but has problems with dates indeed. But we need to run some more tests to be sure about that.

acrylian

Further update: It is indeed the Collator class sorting causing this. While it fixes the original issue of words not being sorting, it fails whenever a date is included. And it does this rather unpredictable which is why we didn't notice it first. We're thinking about how to workaround here,. WE could make an exception for sorting by daate btu then filenames with dates will still fail. So prrobably we will end up with a bunch of new options…

mkanoute

No. The logic that prints images is like this:
while (next_image()):
// print images here
endwhile;

next_image() returns the images in a randon order as decribed before.

acrylian

Yes, of course that is the standard loop on a standard theme. But that's end of the line, The logic of fetching and sorting happens deeper in core.

next_image uses:

• the album class methid getImages() (if we stay with normal album context)`
• which uses the album class method sortImageArray()
• which uses the functions.php function sortByKey()
• which - surprise - uses the functions.php function sortArray() which features the collator class change I am talking about and which I could reprodue the issue with.. For title and description it uses the function sortMultingualArray() which also uses sortArray().

So we need an option respectively options so users can choose which "natural order" they want. Wht works for filenames with dates does not work for say French languagle titles. Again we added this to fix a sorting bug. But becuase of the strange behaviour we missed that it breaks sorting by date. We had test array where all dates were correcty, just some were not without any visible reason.

Or we make a coded exception only for date sorting. But then filenames with a date will still not work properly

Blame it on PHP that offers standard natural order that is not language aware and then introduces an extra class which also does not work completely as expected.

FYI the array comparison of the issue:
https://www.zenphoto.org/test/collator-vs-natcasesort-datesorting.jpg.html

steam4m

Sorry, a bit slow in replying. I checked phpinfo and Intl is enabled, but it looks as though you've discovered the source of the issue (I'm trying to follow but php programming is like speaking in swahili to me).

Is it possible to downgrade a 1.5.8 ro 1.5.7 pending extermination of this bug?

acrylian

While I don't tink we have database changes but I would not recommed it as there are lots of other fixes everywhere.

The hot fix solution is note above to remove the Collator class. There might be afix in 1.5.9a soon, respectively options that are off by default to restore the previous default.

acrylian

Seems we found a better fix than introducing new options by setting a missed parameter.

So please try the 1.5.9a on GitHub to see if it does for you. In this rare case it might work to just copy the functions.php (or even the sortArray() function itself) as there are not many changes from 1.5.8 yet.

mkanoute

I copied functions.php from master on Gitthub and it looks good so far. Thank you!
I'm happy to not have make you lost your time.

acrylian

Thanks for the feedback and being persistent that there was an issue.

steam4m

Thanks, acrylian, likewise copied functions.php and installed it - much appreciated that date sorting is fixed and so far albums look back to normal.

Zenphoto-admin is now telling me that I've changed functions.php and I should run setup. If I do will it revert to older functions.php? (sorry for newb question)

acrylian

Running setup does no revert anything. The notice is just a mild security notice in case some may got in somehow and might have hacked the file. It just checks the filetime in relation to all other files currently. In this case you know you were it.

Thanks for the feedback that the issue is fixed.