Created attachment 4424 [details] [review] Corrections the encoding problem with "recievedlist" If a numbering formula using utf-8 characters is used with a subscription("N° {X}" for example), the "Received issues" list gets encoded incorrectly when the number of received issues is greater than 1. The problem is a double-encoding problem that happens when adding a new issue's number to the content of the recievedlist field, because a utf-8 string gets concatenated with a non-utf8 string. Patch attached that converts the issue number to utf-8 before the concatenation happens.
Created attachment 4472 [details] screenshot The receive screen is ok, but I can reproduce the problem on the subscription summary and edit pages.
Created attachment 4473 [details] [review] [SIGNED-OFF] Converts all the serialseq variables to UTF-8. Corrects a problem when an UTF-8 character is used in the serial numbering formula. The encoding became incorrect when concatenating the number in the subscriptionhistory table. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> To reproduce: - Create subscription with numbering pattern N° {X} - Receive 2 or more issues - Check subscription summary page and manual history fields on the edit screen
Comment on attachment 4424 [details] [review] Corrections the encoding problem with "recievedlist" >From 4106e035be8ac9886aafccbd5284c3ede7cb34c4 Mon Sep 17 00:00:00 2001 >From: =?utf-8?q?Fr=C3=A9d=C3=A9rick=20Capovilla?= <frederick.capovilla@sys-tech.net> >Date: Tue, 7 Jun 2011 14:03:08 -0400 >Subject: [PATCH] Converts all the serialseq variables to UTF-8. > >Corrects a problem when an UTF-8 character is used in the serial >numbering formula. The encoding became incorrect when concatenating the >number in the subscriptionhistory table. >--- > serials/serials-edit.pl | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > >diff --git a/serials/serials-edit.pl b/serials/serials-edit.pl >index aaa0574..b486c04 100755 >--- a/serials/serials-edit.pl >+++ b/serials/serials-edit.pl >@@ -196,6 +196,11 @@ $template->param( subscriptions => \@subscriptionloop ); > > if ( $op and $op eq 'serialchangestatus' ) { > >+ # Convert serialseqs to UTF-8 to prevent encoding problems >+ foreach my $seq (@serialseqs) { >+ utf8::decode($seq) unless utf8::is_utf8($seq); >+ } >+ > my $newserial; > for ( my $i = 0 ; $i <= $#serialids ; $i++ ) { > >-- >1.5.6.5 >
Created attachment 4969 [details] [review] proposed patch (passed QA) QA comment * The SIGNED-OFF patch does not apply with a strange message : fatal: cannot convert from UTF-8utf-8 to UTF-8 Looking at it I can see : Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="utf-8" If I remove the 3rd line, patch applies cleanly now. Katrin, it seems the 3rd line has been added by your format patch. Any idea why ? * The patch fixes a really annoying problem. * Works as expected. I had some problems to reproduce the bad behaviour due to a missing information = manualhistory must be set to 0 (unchecked) to see it ! * I still don't understand why we must sometimes explicitly decode utf8... Marking passed QA
No idea Paul, but seems to be a one time problem. Other patches signed-off by me seem to be ok.
The comment here is misleading, we are attempting to decode from UTF-X to utf8::decode($string) Attempts to convert in-place the octet sequence in UTF-X to the corresponding character sequence. The UTF-8 flag is turned on only if the source string contains multiple-byte UTF-X characters. If $string is invalid as UTF-X, returns false; otherwise returns true. Note that this function does not handle arbitrary encodings. Therefore Encode is recommended for the general purposes; see also Encode. Frédérick are we sure we want to use decode here, not encode?
(In reply to comment #6) > The comment here is misleading, we are attempting to decode from UTF-X to > > utf8::decode($string) > Frédérick are we sure we want to use decode here, not encode? That's a part of the problem I have with utf8 encoding/decoding/... = I don't understand why this string is needed, I just can confirm it works. There is another bug (about itemtypes with diacritics in description) that is also related to this utf8:decode. utf8 handling in Perl is still a mystery for me :(
It's been a while since I created that patch but here is what I understand : I remember that in the NewIssue subroutine of Serials.pm, The content of $serialseq is concatenated with a variable fetched from a SQL query and that's where the problem happen. I did some tests to check the utf-8 flag on those two variables $serialseq = variable from the form (is_utf8 = false) $recievedlist = variable from SQL (is_utf8 = true) The encoding of the data fetched from SQL differs from the encoding of the data received from the form. If two string with a different encoding gets concatenated together, the encoding of one of the string is automatically changed, and we get an encoding problem on one half of the string. Using decode on $serialseq adds the utf-8 flag, so we don't get an encoding problem when we concatenate. We don't get this encoding problem when the first item is added in $recievedlist because $recievedlist is empty and doesn't automatically get the utf-8 flag. I'm guessing Perl DBI automatically addes the utf-8 flag if it finds utf-8 characters in a string returned from a SELECT. Anyways, that's what I think is happening. Correct me if I'm wrong?
chris, this has passed QA, the answer from Frédérick sounds logical, I think you can push.
Pushed, please test
The fix for this bug was published in the 3.4.5 release. If you were the reporter of this bug, please take time to verify the fix and update the status of this bug report accordingly. If the bug is fixed to your satisfaction, please close this report.
There have been no further reports of problems so I am marking this bug resolved.