Bug 23488 - Line wrapping doesn't always respect word order in Patron card creator
Summary: Line wrapping doesn't always respect word order in Patron card creator
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Maryse Simard
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-21 21:26 UTC by Maryse Simard
Modified: 2020-11-30 21:47 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.05, 19.05.10


Attachments
Bug 23488: Line wrapping doesn't always respect word order in Patron card creator (2.81 KB, patch)
2019-08-21 21:28 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 23488: Line wrapping doesn't always respect word order in Patron card creator (1.89 KB, patch)
2020-01-08 16:30 UTC, Gabriel DeCarufel
Details | Diff | Splinter Review
Bug 23488: Line wrapping doesn't always respect word order in Patron card creator (1.95 KB, patch)
2020-01-08 21:52 UTC, William Frazilien
Details | Diff | Splinter Review
Bug 23488: Line wrapping doesn't always respect word order in Patron card creator (2.97 KB, patch)
2020-02-17 16:16 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 23488: Line wrapping doesn't always respect word order in Patron card creator (3.14 KB, patch)
2020-02-29 21:30 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Maryse Simard 2019-08-21 21:26:51 UTC
In patron card creator, layouts can define up to three lines of text to print to the card. When the text of a line is too long, wrapping will happen. If the wrapped text contains a word that is repeated at least once in the string, the words in the text can be rearranged, the repeated words being grouped together.

To reproduce, I use the following text that I put in a card layout : "one two three one two three one two three one two three one two three one two three one two three one two three" I then export some patron cards using this layout. In the resulting file, the words are all out of order : all of the ones appear first, followed by all the twos and only then the threes.

The problem is that wrapping happens by finding the last word that can be cut from the string and putting it in a new line. But, when the word is then removed from the original text, only the first occurrence is removed if it appears more than once. The new last word to be removed from the end of the line is
the same word again and again until there is none left.

We should make sure to remove the word at the end of the line, exactly where we found it to start with.
Comment 1 Maryse Simard 2019-08-21 21:28:02 UTC
Created attachment 92411 [details] [review]
Bug 23488: Line wrapping doesn't always respect word order in Patron card creator

When wrapping long lines of text, the line is divided by removing each
word from the end of the line and putting it in a new one until the line
is the right width. When the word to be removed appears multiple time
in the line, it is not the last occurrence that is removed.

This patch changes the regular expression used to remove the part of
the text that is wrapped to a new line, making sure it removes it at
the end of the text.

Test plan:

1. Go to Tools > Patron card creator

2. Have a card template and a card batch
    -> If needs be, you can create them by using
       New > Card template or New > Card batch

3. Create a layout and use one text field containing a long text with
    at least one word which is repeated a minimum of 2 times
    (preferably towrdds the end of the text, since it has to be picked
    as one of the words to appear in the new line). You can use this:

        one two three one two three one two three one two three
        one two three one two three one two three one two three ...

4. Go to Manage > Card batches and export a batch

5. Choose the layout set up in 3.

6. Click the Export button and open the resulting pdf file

7. Notice all the repeated word have been grouped
    -> For this example : all of the ones appear first, followed by
       all the twos and only then the threes.

8. Apply patch

9. Repeat step 4 through 7
    => this time the order of the words has not changed!
Comment 2 Gabriel DeCarufel 2020-01-08 16:30:13 UTC
Created attachment 97030 [details] [review]
Bug 23488: Line wrapping doesn't always respect word order in Patron card creator

When wrapping long lines of text, the line is divided by removing each
word from the end of the line and putting it in a new one until the line
is the right width. When the word to be removed appears multiple time
in the line, it is not the last occurrence that is removed.

This patch changes the regular expression used to remove the part of
the text that is wrapped to a new line, making sure it removes it at
the end of the text.

Signed-off-by: Gabriel DeCarufel <gabriel@inlibro.com>
Comment 3 William Frazilien 2020-01-08 21:52:14 UTC
Created attachment 97059 [details] [review]
Bug 23488: Line wrapping doesn't always respect word order in Patron card creator

When wrapping long lines of text, the line is divided by removing each
word from the end of the line and putting it in a new one until the line
is the right width. When the word to be removed appears multiple time
in the line, it is not the last occurrence that is removed.

This patch changes the regular expression used to remove the part of
the text that is wrapped to a new line, making sure it removes it at
the end of the text.

Signed-off-by: Gabriel DeCarufel <gabriel@inlibro.com>
Signed-off-by: William Frazilien <william.frazilien@inlibro.com>
Comment 4 Bernardo Gonzalez Kriegel 2020-02-17 16:16:01 UTC
Created attachment 99115 [details] [review]
Bug 23488: Line wrapping doesn't always respect word order in Patron card creator

When wrapping long lines of text, the line is divided by removing each
word from the end of the line and putting it in a new one until the line
is the right width. When the word to be removed appears multiple time
in the line, it is not the last occurrence that is removed.

This patch changes the regular expression used to remove the part of
the text that is wrapped to a new line, making sure it removes it at
the end of the text.

Test plan:

1. Go to Tools > Patron card creator

2. Have a card template and a card batch
    -> If needs be, you can create them by using
       New > Card template or New > Card batch

3. Create a layout and use one text field containing a long text with
    at least one word which is repeated a minimum of 2 times
    (preferably towrdds the end of the text, since it has to be picked
    as one of the words to appear in the new line). You can use this:

        one two three one two three one two three one two three
        one two three one two three one two three one two three ...

4. Go to Manage > Card batches and export a batch

5. Choose the layout set up in 3.

6. Click the Export button and open the resulting pdf file

7. Notice all the repeated word have been grouped
    -> For this example : all of the ones appear first, followed by
       all the twos and only then the threes.

8. Apply patch

9. Repeat step 4 through 7
    => this time the order of the words has not changed!

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described, no errors.
Tested adding addrees to test layout, and using the proposed
text.
Comment 5 Katrin Fischer 2020-02-29 21:29:34 UTC
Readded the 2 sign-off lines that got lost when the test plan was added to the commit message (thx for that Bernardo!)
Comment 6 Katrin Fischer 2020-02-29 21:30:29 UTC
Created attachment 99835 [details] [review]
Bug 23488: Line wrapping doesn't always respect word order in Patron card creator

When wrapping long lines of text, the line is divided by removing each
word from the end of the line and putting it in a new one until the line
is the right width. When the word to be removed appears multiple time
in the line, it is not the last occurrence that is removed.

This patch changes the regular expression used to remove the part of
the text that is wrapped to a new line, making sure it removes it at
the end of the text.

Test plan:

1. Go to Tools > Patron card creator

2. Have a card template and a card batch
    -> If needs be, you can create them by using
       New > Card template or New > Card batch

3. Create a layout and use one text field containing a long text with
    at least one word which is repeated a minimum of 2 times
    (preferably towrdds the end of the text, since it has to be picked
    as one of the words to appear in the new line). You can use this:

        one two three one two three one two three one two three
        one two three one two three one two three one two three ...

4. Go to Manage > Card batches and export a batch

5. Choose the layout set up in 3.

6. Click the Export button and open the resulting pdf file

7. Notice all the repeated word have been grouped
    -> For this example : all of the ones appear first, followed by
       all the twos and only then the threes.

8. Apply patch

9. Repeat step 4 through 7
    => this time the order of the words has not changed!

Signed-off-by: Gabriel DeCarufel <gabriel@inlibro.com>
Signed-off-by: William Frazilien <william.frazilien@inlibro.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described, no errors.
Tested adding addrees to test layout, and using the proposed
text.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Martin Renvoize 2020-03-02 09:47:19 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 8 Joy Nelson 2020-04-01 20:44:08 UTC
Pushed to 19.11.x branch for 19.11.05
Comment 9 Lucas Gass 2020-04-08 22:02:32 UTC
backported to 19.05.x for 19.05.10
Comment 10 Hayley Pelham 2020-04-15 23:11:40 UTC
Minor patch will not be backported to 18.11.x series.