After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 770695 - Wrong file order in .cbz
Wrong file order in .cbz
Status: RESOLVED FIXED
Product: evince
Classification: Core
Component: general
3.20.x
Other Linux
: Normal normal
: ---
Assigned To: Evince Maintainers
Evince Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-09-01 07:28 UTC by Filippo Berto
Modified: 2016-10-05 13:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
comics: Sort pages in natural order (1.17 KB, patch)
2016-09-09 15:06 UTC, Felipe Borges
none Details | Review
comics: Sort pages in natural order (1.07 KB, patch)
2016-09-30 13:43 UTC, Felipe Borges
none Details | Review
comics: Sort pages in natural order (1.21 KB, patch)
2016-10-05 12:22 UTC, Felipe Borges
committed Details | Review

Description Filippo Berto 2016-09-01 07:28:04 UTC
When opening a .cbz file it shows the images in the wrong order:

ex. 
file contains
   1.jpg
   ...
   20.jpg

program shows
   1.jpg
   10.jpg
   11.jpg
   ...
   2.jpg
   20.jpg
   21.jpg

   ...

The way file are sorted is alphabetical and not numerical.
Comment 1 Felipe Borges 2016-09-09 15:06:18 UTC
Created attachment 335203 [details] [review]
comics: Sort pages in natural order

Evince was sorting pages in lexicographic order. In doing so, often
files named page1, page2, ..., page9, page10, ... to be sorted as
page1, page10, ..., page2.

This patch introduces the usage of strverscmp(). This function is
missing on all non-glibc platforms.
Comment 2 José Aliste 2016-09-09 16:24:49 UTC
Do you mean that this makes evince stop compiling on Windows? I know we don't distribute evince on windows but it is still available in mingw through msys2.
Comment 3 Felipe Borges 2016-09-09 18:25:13 UTC
(In reply to José Aliste from comment #2)
> Do you mean that this makes evince stop compiling on Windows? I know we
> don't distribute evince on windows but it is still available in mingw
> through msys2.

I have never tested in these platforms, but if you feel better I can also:
1. conditionally use it in supported platforms; OR
2. implement our own natural sort.
Comment 4 Germán Poo-Caamaño 2016-09-28 19:36:46 UTC
I am inclined for 1.
Comment 5 Felipe Borges 2016-09-30 13:43:24 UTC
(In reply to Germán Poo-Caamaño from comment #4)
> I am inclined for 1.

I did some research and according to https://www.gnu.org/software/gnulib/manual/html_node/strverscmp.html, the problem seems to be fixed by now.

The list of gnulib supported platforms is https://www.gnu.org/software/gnulib/manual/html_node/Target-Platforms.html

I am not able to test it on different platforms, but I'm reattaching the patch with a newer commit message (dropping the portability comment, if that's the case).
Comment 6 Felipe Borges 2016-09-30 13:43:37 UTC
Created attachment 336671 [details] [review]
comics: Sort pages in natural order

Evince was sorting pages in lexicographic order. In doing so, often
files named page1, page2, ..., page9, page10, ... to be sorted as
page1, page10, ..., page2.
Comment 7 Felipe Borges 2016-10-03 15:43:53 UTC
Attachment 336671 [details] pushed as 3977c03 - comics: Sort pages in natural order
Comment 8 Ting-Wei Lan 2016-10-05 10:27:05 UTC
This causes build failure on FreeBSD:
backend/comics/comics-document.c:470:10: error: implicit declaration of function 'strverscmp' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

(In reply to Felipe Borges from comment #5)
> (In reply to Germán Poo-Caamaño from comment #4)
> > I am inclined for 1.
> 
> I did some research and according to
> https://www.gnu.org/software/gnulib/manual/html_node/strverscmp.html, the
> problem seems to be fixed by now.
> 
> The list of gnulib supported platforms is
> https://www.gnu.org/software/gnulib/manual/html_node/Target-Platforms.html
> 
> I am not able to test it on different platforms, but I'm reattaching the
> patch with a newer commit message (dropping the portability comment, if
> that's the case).

Evince doesn't use gnulib, so the portability problem cannot be fixed by gnulib.
Comment 9 Ting-Wei Lan 2016-10-05 10:40:22 UTC
It seems that nautilus uses g_utf8_collate_key_for_filename() to avoid portability problems.
Comment 10 Felipe Borges 2016-10-05 11:55:44 UTC
(In reply to Ting-Wei Lan from comment #9)
> It seems that nautilus uses g_utf8_collate_key_for_filename() to avoid
> portability problems.

great. I will revert the commit and propose another patch using what you proposed.
Comment 11 Felipe Borges 2016-10-05 12:22:38 UTC
Created attachment 336981 [details] [review]
comics: Sort pages in natural order

Use g_utf8_collate_key_for_filename to convert the page filenames
into collation keys in order to compare using strcmp().

This prevents pages named such as page1, page2, page10, to be
sorted as page1, page10, page2...
Comment 12 Ting-Wei Lan 2016-10-05 12:52:16 UTC
Thanks, attachment 336981 [details] [review] fixes the build error.
Comment 13 Felipe Borges 2016-10-05 13:23:49 UTC
Attachment 336981 [details] pushed as 3cace57 - comics: Sort pages in natural order