[Bug 14468] New: Remove warnings when creating Labels
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Bug ID: 14468 Summary: Remove warnings when creating Labels Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Label/patron card printing Assignee: cnighswonger@foundations.edu Reporter: bgkriegel@gmail.com QA Contact: testopia@bugs.koha-community.org There are a lot of warnings when printing labels -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 --- Comment #1 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 40699 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40699&action=edit Bug 14468: Remove warnings when creating Labels This patch removes 2 types of warnings when creating Labels a) Using '0' as filling char is not to the like of prStrWidth On C4/Creators/Layout.pm b) A RM followup from Galen on Bug 8375 to C4/Creators/PDF.pm makes impossible to find properly a font :( (http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=f7ef93e758850e99...) on C4/Labels/Label.pm To test: Pre patch 1) Go to Tools > Labels 2) Create a new layout, name ABC, type 'Barcode/Biblio' 3) Create a new batch, add 2 items 4) Export as PDF using the new layout 5) Look at logs, you will find something like a) label-create-pdf.pl: Use of uninitialized value $strwidth in numeric lt (<) at /home/bgkriegel/kohaclone/C4/Creators/Layout.pm line 233., referer: http://staffdev.koha-community.org.ar/cgi-bin/koha/labels/label-print.pl (Only one of this kind) b) Two related lines similar to this examples label-create-pdf.pl: Use of uninitialized value in -e at /home/bgkriegel/kohaclone/C4/Creators/PDF.pm line 226., referer: http://staffdev.koha-community.org.ar/cgi-bin/koha/labels/label-print.pl label-create-pdf.pl: ERROR in koha-conf.xml -- missing <font type="Ft1">/path/to/font.ttf</font> at /home/bgkriegel/kohaclone/C4/Creators/PDF.pm line 229., referer: http://staffdev.koha-community.org.ar/cgi-bin/koha/labels/label-print.pl (many many lines x number of items on batch) 6) Apply the patch 7) Export the same PDF, no more warnings 8) Bonus 1: change main font to Courier and Helvetica and check results 9) Bonus 2: check using arabic records Many hours to find :( Easy to fix -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Assignee|cnighswonger@foundations.ed |bgkriegel@gmail.com |u | Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Chris Nighswonger <cnighswonger@foundations.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |cnighswonger@foundations.ed | |u --- Comment #2 from Chris Nighswonger <cnighswonger@foundations.edu> --- There will most likely be problems removing the use of internal fonts for those who cannot or do not desire to use TTFs. A better solution would be to incorporate the possibility of both. Ie. Check to see if TTFs are configured and fall back to internal fonts if not. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 --- Comment #3 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- (In reply to Chris Nighswonger from comment #2)
There will most likely be problems removing the use of internal fonts for those who cannot or do not desire to use TTFs. A better solution would be to incorporate the possibility of both. Ie. Check to see if TTFs are configured and fall back to internal fonts if not.
Yes, but that is what C4::Creators::PDF->Font does ATM. sub Font { my $self = shift; my $fontName = shift; my $ttf = C4::Context->config('ttf'); if ( $ttf ) { my $ttf_path = first { $_->{type} eq $fontName } @{ $ttf->{font} }; if ( -e $ttf_path->{content} ) { return prTTFont($ttf_path->{content}); } else { warn "ERROR in koha-conf.xml -- missing <font type=\"$fontName\">/path/to/font.ttf</font>"; } } return prFont($fontName); } It returns prTTFont or prFont using C4::Context->config('ttf') Just tried with ttf 'disabled' I get pdffonts label_batch_1.pdf name type encoding emb sub uni object ID ------------------------------------ ----------------- ---------------- --- --- --- --------- Courier Type 1 WinAnsi no no no 5 0 Helvetica Type 1 WinAnsi no no no 4 0 With ttf enabled pdffonts label_batch_1.pdf name type encoding emb sub uni object ID ------------------------------------ ----------------- ---------------- --- --- --- --------- Courier Type 1 WinAnsi no no no 9 0 BXCJIM+DejaVuSans CID TrueType Identity-H yes yes no 4 0 Courier is used below barcode, the other is for text. It's working :) (but without ttf not for complex scripts) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 --- Comment #4 from Chris Nighswonger <cnighswonger@foundations.edu> --- (In reply to Bernardo Gonzalez Kriegel from comment #3)
(In reply to Chris Nighswonger from comment #2)
There will most likely be problems removing the use of internal fonts for those who cannot or do not desire to use TTFs. A better solution would be to incorporate the possibility of both. Ie. Check to see if TTFs are configured and fall back to internal fonts if not.
Yes, but that is what C4::Creators::PDF->Font does ATM.
I realize that. However, it appears that you remove the calls to that in your patch which is what triggered my concern. I apologize if I'm missing something since I'm really not focused at the moment, but in any case, as long as things still work for someone who elects not to install TTFs on their system, I'm fine with the fix. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 --- Comment #5 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- (In reply to Chris Nighswonger from comment #4)
I realize that. However, it appears that you remove the calls to that in your patch which is what triggered my concern.
Yes, I remove a call in C4/Labels/Label.pm, but add explicitly one on C4/Creators/PDF.pm replacing Galen's code that do almost the same. If we call it on Label.pm and again in PDF.pm then we have all those warnings in the logs.
I apologize if I'm missing something since I'm really not focused at the moment, but in any case, as long as things still work for someone who elects not to install TTFs on their system, I'm fine with the fix.
No problem :) I tried that before your first comment and works fine without TTF fonts. The problem only affects sites with TTF fonts on config. Originally we transformed 'TR' to 'Ft1' on Labels.pm (using Font function) then we searched 'Ft1' inside TTF fonts in StrWidth (using Galen's code) and spit a warning because it was not found. Now I send 'TR' to StrWidth, it calls Font sub and get 'Ft1', that is passed to prStrWidth (works the same with or without TTF) That's the reason to remove the call on Label.pm Regards -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 --- Comment #6 from Chris Nighswonger <cnighswonger@foundations.edu> --- That all sounds good. I wish I had time to sign off! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Nick Clemens <nick@quecheelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Nick Clemens <nick@quecheelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40699|0 |1 is obsolete| | --- Comment #7 from Nick Clemens <nick@quecheelibrary.org> --- Created attachment 40741 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40741&action=edit [SIGNED OFF] Bug 14468: Remove warnings when creating Labels This patch removes 2 types of warnings when creating Labels a) Using '0' as filling char is not to the like of prStrWidth On C4/Creators/Layout.pm b) A RM followup from Galen on Bug 8375 to C4/Creators/PDF.pm makes impossible to find properly a font :( (http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=f7ef93e758850e99...) on C4/Labels/Label.pm To test: Pre patch 1) Go to Tools > Labels 2) Create a new layout, name ABC, type 'Barcode/Biblio' 3) Create a new batch, add 2 items 4) Export as PDF using the new layout 5) Look at logs, you will find something like a) label-create-pdf.pl: Use of uninitialized value $strwidth in numeric lt (<) at /home/bgkriegel/kohaclone/C4/Creators/Layout.pm line 233., referer: http://staffdev.koha-community.org.ar/cgi-bin/koha/labels/label-print.pl (Only one of this kind) b) Two related lines similar to this examples label-create-pdf.pl: Use of uninitialized value in -e at /home/bgkriegel/kohaclone/C4/Creators/PDF.pm line 226., referer: http://staffdev.koha-community.org.ar/cgi-bin/koha/labels/label-print.pl label-create-pdf.pl: ERROR in koha-conf.xml -- missing <font type="Ft1">/path/to/font.ttf</font> at /home/bgkriegel/kohaclone/C4/Creators/PDF.pm line 229., referer: http://staffdev.koha-community.org.ar/cgi-bin/koha/labels/label-print.pl (many many lines x number of items on batch) 6) Apply the patch 7) Export the same PDF, no more warnings 8) Bonus 1: change main font to Courier and Helvetica and check results 9) Bonus 2: check using arabic records Many hours to find :( Easy to fix Signed-off-by: Nick Clemens <nick@quecheelibrary.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Nick Clemens <nick@quecheelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@quecheelibrary.org --- Comment #8 from Nick Clemens <nick@quecheelibrary.org> --- Bonus 1 - Works! Bonus 2 - I only tried with pasting arabic text in as call number but it displayed nicely in PDF which I think is an improvement -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Chris Nighswonger from comment #6)
That all sounds good. I wish I had time to sign off!
Chris, I am not confident with part of code, it would be great to get your signoff on this one :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 --- Comment #10 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- (In reply to Jonathan Druart from comment #9)
(In reply to Chris Nighswonger from comment #6)
That all sounds good. I wish I had time to sign off!
Chris, I am not confident with part of code, it would be great to get your signoff on this one :)
Hi Jonathan, while waiting to Chris, which part of the code is giving you trouble? I can review it if you want -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Bernardo Gonzalez Kriegel from comment #10)
(In reply to Jonathan Druart from comment #9)
(In reply to Chris Nighswonger from comment #6)
That all sounds good. I wish I had time to sign off!
Chris, I am not confident with part of code, it would be great to get your signoff on this one :)
Hi Jonathan, while waiting to Chris, which part of the code is giving you trouble? I can review it if you want
Hum, all C4::Creators? :) It works as expected but I cannot be sure the patch won't introduce regressions. Looking at it again and all looks good. Marked as Passed QA. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #40741|0 |1 is obsolete| | --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 41375 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41375&action=edit Bug 14468: Remove warnings when creating Labels This patch removes 2 types of warnings when creating Labels a) Using '0' as filling char is not to the like of prStrWidth On C4/Creators/Layout.pm b) A RM followup from Galen on Bug 8375 to C4/Creators/PDF.pm makes impossible to find properly a font :( (http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=f7ef93e758850e99...) on C4/Labels/Label.pm To test: Pre patch 1) Go to Tools > Labels 2) Create a new layout, name ABC, type 'Barcode/Biblio' 3) Create a new batch, add 2 items 4) Export as PDF using the new layout 5) Look at logs, you will find something like a) label-create-pdf.pl: Use of uninitialized value $strwidth in numeric lt (<) at /home/bgkriegel/kohaclone/C4/Creators/Layout.pm line 233., referer: http://staffdev.koha-community.org.ar/cgi-bin/koha/labels/label-print.pl (Only one of this kind) b) Two related lines similar to this examples label-create-pdf.pl: Use of uninitialized value in -e at /home/bgkriegel/kohaclone/C4/Creators/PDF.pm line 226., referer: http://staffdev.koha-community.org.ar/cgi-bin/koha/labels/label-print.pl label-create-pdf.pl: ERROR in koha-conf.xml -- missing <font type="Ft1">/path/to/font.ttf</font> at /home/bgkriegel/kohaclone/C4/Creators/PDF.pm line 229., referer: http://staffdev.koha-community.org.ar/cgi-bin/koha/labels/label-print.pl (many many lines x number of items on batch) 6) Apply the patch 7) Export the same PDF, no more warnings 8) Bonus 1: change main font to Courier and Helvetica and check results 9) Bonus 2: check using arabic records Many hours to find :( Easy to fix Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Passed QA |Failed QA --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Hi, please check why this patch makes tests fail: $ prove t/db_dependent/Labels/t_Layout.t t/db_dependent/Labels/t_Layout.t .. 1/36 # Testing Layout->new() method. # Testing Layout->get_attr() method. # Testing Layout->set_attr() method. # Testing Layout->save() method with a new object. # Testing Layout->retrieve() method. # Testing Layout->save() method with an updated object. # Testing Layout->get_text_wrap_cols() method. # Failed test 'Layout->get_text_wrap_cols()' # at t/db_dependent/Labels/t_Layout.t line 103. # got: '21' # expected: '23' # Testing Layout->delete() method. Use of uninitialized value $del_results in string ne at t/db_dependent/Labels/t_Layout.t line 108. # Looks like you failed 1 test of 36. ... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Theodoros Theodoropoulos <theod@lib.auth.gr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |theod@lib.auth.gr -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 --- Comment #14 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 43882 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43882&action=edit Bug 14468: followup to fix tests This patch fix an expected test result. Also changes a comparison for the return value to 'delete' layout sub, in case of success it returns 'undef' and triggers the message 'use of uninitialized value $del_results ...' To test: 1) Whitout this patch prove t/db_dependent/Labels/t_Layout.t fails. Also note the message "Use of uninitialized value $del_results in string ne at t/db_dependent/Labels/t_Layout.t line 110." for using the return value of delete layout (undef on success!) 2) Apply the patch 3) Test pass and no more message -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #15 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Fixed test, changed to needs signoff. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #43882|0 |1 is obsolete| | --- Comment #16 from Marc Véron <veron@veron.ch> --- Created attachment 44015 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44015&action=edit Bug 14468: followup to fix tests This patch fix an expected test result. Also changes a comparison for the return value to 'delete' layout sub, in case of success it returns 'undef' and triggers the message 'use of uninitialized value $del_results ...' To test: 1) Whitout this patch prove t/db_dependent/Labels/t_Layout.t fails. Also note the message "Use of uninitialized value $del_results in string ne at t/db_dependent/Labels/t_Layout.t line 110." for using the return value of delete layout (undef on success!) 2) Apply the patch 3) Test pass and no more message Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44015|0 |1 is obsolete| | --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 44045 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44045&action=edit Bug 14468: followup to fix tests This patch fix an expected test result. Also changes a comparison for the return value to 'delete' layout sub, in case of success it returns 'undef' and triggers the message 'use of uninitialized value $del_results ...' To test: 1) Whitout this patch prove t/db_dependent/Labels/t_Layout.t fails. Also note the message "Use of uninitialized value $del_results in string ne at t/db_dependent/Labels/t_Layout.t line 110." for using the return value of delete layout (undef on success!) 2) Apply the patch 3) Test pass and no more message Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 44063 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44063&action=edit Bug 14468: (QA followup) remove useless diags Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14468 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Parche aplicado en master. ¡Gracias Bernardo! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org