Bugzilla – Attachment 39739 Details for
Bug 14308
Remove CGI::checkbox from subfield editors
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
proposed patch
0001-Bug-14308-Remove-CGI-checkbox-from-subfield-editors.patch (text/plain), 23.02 KB, created by
Bernardo Gonzalez Kriegel
on 2015-06-01 12:53:54 UTC
(
hide
)
Description:
proposed patch
Filename:
MIME Type:
Creator:
Bernardo Gonzalez Kriegel
Created:
2015-06-01 12:53:54 UTC
Size:
23.02 KB
patch
obsolete
>From 84a2ad874ed005ea913ff82173088fe1553179b4 Mon Sep 17 00:00:00 2001 >From: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> >Date: Mon, 1 Jun 2015 09:28:40 -0300 >Subject: [PATCH] Bug 14308: Remove CGI::checkbox from subfield editors > >This patch removes use of CGI::checkbox from >frameworks subfield editors. >Also removes use of CGI::escapeHTML > >To test: >1) Apply the patch >2) Go to bibliographic frameworks, edit a field with subfields >3) Check for regressions on Edit/Save/Add subfields >i.e. >a. Edit subfield with known values on repeatable, mandatory, isurl checkbox >b. Change values, save, edit again and verify values. > >4) Go to authority frameworks, edit a field with subfields >5) Check for regressions on Edit/Save/Add subfields >--- > admin/auth_subfields_structure.pl | 69 +++++++++--------- > admin/marc_subfields_structure.pl | 84 ++++++++++------------ > .../en/modules/admin/auth_subfields_structure.tt | 43 ++++++++--- > .../en/modules/admin/marc_subfields_structure.tt | 52 ++++++++++---- > 4 files changed, 143 insertions(+), 105 deletions(-) > >diff --git a/admin/auth_subfields_structure.pl b/admin/auth_subfields_structure.pl >index 3833b48..b30a46e 100755 >--- a/admin/auth_subfields_structure.pl >+++ b/admin/auth_subfields_structure.pl >@@ -141,9 +141,9 @@ if ($op eq 'add_form') { > }; > $row_data{tagsubfieldinput} = "<input type=\"hidden\" name=\"tagsubfield\" value=\"".$data->{'tagsubfield'}."\" id=\"tagsubfield\" />"; > $row_data{tagsubfield} = $data->{'tagsubfield'}; >- $row_data{liblibrarian} = CGI::escapeHTML($data->{'liblibrarian'}); >- $row_data{libopac} = CGI::escapeHTML($data->{'libopac'}); >- $row_data{seealso} = CGI::escapeHTML($data->{'seealso'}); >+ $row_data{liblibrarian} = $data->{'liblibrarian'}; >+ $row_data{libopac} = $data->{'libopac'}; >+ $row_data{seealso} = $data->{'seealso'}; # FIXME: this parameter is not used on template > $row_data{kohafield} = { > id => "kohafield$i", > values => \@kohafields, >@@ -164,23 +164,22 @@ if ($op eq 'add_form') { > values => \@value_builder, > default => $data->{'value_builder'}, > }; >- >- $row_data{repeatable} = CGI::checkbox(-name=>"repeatable$i", >- -checked => $data->{'repeatable'}?'checked':'', >- -value => 1, >- -label => '', >- -id => "repeatable$i"); >- $row_data{mandatory} = CGI::checkbox(-name => "mandatory$i", >- -checked => $data->{'mandatory'}?'checked':'', >- -value => 1, >- -label => '', >- -id => "mandatory$i"); >- $row_data{hidden} = CGI::escapeHTML($data->{hidden}) ; >- $row_data{isurl} = CGI::checkbox( -name => "isurl$i", >- -id => "isurl$i", >- -checked => $data->{'isurl'}?'checked':'', >- -value => 1, >- -label => ''); >+ $row_data{repeatable} = { >+ name => "repeatable$i", >+ id => "repeatable$i", >+ checked => $data->{'repeatable'}?'checked':'', >+ }; >+ $row_data{mandatory} = { >+ name => "mandatory$i", >+ id => "mandatory$i", >+ checked => $data->{'mandatory'}?'checked':'', >+ }; >+ $row_data{hidden} = $data->{hidden}; # FIXME: this parameter is not used on template >+ $row_data{isurl} = { >+ name => "isurl$i", >+ id => "isurl$i", >+ checked => $data->{'isurl'}?'checked':'', >+ }; > $row_data{row} = $i; > push(@loop_data, \%row_data); > $i++; >@@ -208,21 +207,21 @@ if ($op eq 'add_form') { > $row_data{libopac} = ""; > $row_data{seealso} = ""; > $row_data{hidden} = "000"; >- $row_data{repeatable} = CGI::checkbox( -name=> 'repeatable', >- -id => "repeatable$i", >- -checked => '', >- -value => 1, >- -label => ''); >- $row_data{mandatory} = CGI::checkbox( -name=> 'mandatory', >- -id => "mandatory$i", >- -checked => '', >- -value => 1, >- -label => ''); >- $row_data{isurl} = CGI::checkbox(-name => 'isurl', >- -id => "isurl$i", >- -checked => '', >- -value => 1, >- -label => ''); >+ $row_data{repeatable} = { >+ name => "repeatable$i", >+ id => "repeatable$i", >+ checked => '', >+ }; >+ $row_data{mandatory} = { >+ name => "mandatory$i", >+ id => "mandatory$i", >+ checked => '', >+ }; >+ $row_data{isurl} = { >+ name => "isurl$i", >+ id => "isurl$i", >+ checked => '', >+ }; > $row_data{kohafield} = { > id => "kohafield$i", > values => \@kohafields, >diff --git a/admin/marc_subfields_structure.pl b/admin/marc_subfields_structure.pl >index c8b7815..8b83ceb 100755 >--- a/admin/marc_subfields_structure.pl >+++ b/admin/marc_subfields_structure.pl >@@ -193,9 +193,9 @@ if ( $op eq 'add_form' ) { > . "\" id=\"tagsubfield\" />"; > $row_data{subfieldcode} = $data->{'tagsubfield'} eq '@'?'_':$data->{'tagsubfield'}; > $row_data{urisubfieldcode} = $row_data{subfieldcode} eq '%' ? 'pct' : $row_data{subfieldcode}; >- $row_data{liblibrarian} = CGI::escapeHTML( $data->{'liblibrarian'} ); >- $row_data{libopac} = CGI::escapeHTML( $data->{'libopac'} ); >- $row_data{seealso} = CGI::escapeHTML( $data->{'seealso'} ); >+ $row_data{liblibrarian} = $data->{'liblibrarian'}; >+ $row_data{libopac} = $data->{'libopac'}; >+ $row_data{seealso} = $data->{'seealso'}; > $row_data{kohafield} = { > id => "kohafield$i", > values => \@kohafields, >@@ -216,30 +216,24 @@ if ( $op eq 'add_form' ) { > values => \@authtypes, > default => $data->{'authtypecode'}, > }; >- $row_data{repeatable} = CGI::checkbox( >- -name => "repeatable$i", >- -checked => $data->{'repeatable'} ? 'checked' : '', >- -value => 1, >- -label => '', >- -id => "repeatable$i" >- ); >- $row_data{mandatory} = CGI::checkbox( >- -name => "mandatory$i", >- -checked => $data->{'mandatory'} ? 'checked' : '', >- -value => 1, >- -label => '', >- -id => "mandatory$i" >- ); >- $row_data{hidden} = CGI::escapeHTML( $data->{hidden} ); >- $row_data{isurl} = CGI::checkbox( >- -name => "isurl$i", >- -id => "isurl$i", >- -checked => $data->{'isurl'} ? 'checked' : '', >- -value => 1, >- -label => '' >- ); >+ $row_data{repeatable} = { >+ name => "repeatable$i", >+ id => "repeatable$i", >+ checked => $data->{'repeatable'} ? 'checked' : '', >+ }; >+ $row_data{mandatory} = { >+ name => "mandatory$i", >+ id => "mandatory$i", >+ checked => $data->{'mandatory'} ? 'checked' : '', >+ }; >+ $row_data{hidden} = $data->{hidden}; >+ $row_data{isurl} = { >+ name => "isurl$i", >+ id => "isurl$i", >+ checked => $data->{'isurl'} ? 'checked' : '', >+ }; > $row_data{row} = $i; >- $row_data{link} = CGI::escapeHTML( $data->{'link'} ); >+ $row_data{link} = $data->{'link'}; > push( @loop_data, \%row_data ); > $i++; > } >@@ -267,27 +261,21 @@ if ( $op eq 'add_form' ) { > default => "$data->{'kohafield'}", > }; > $row_data{hidden} = ""; >- $row_data{repeatable} = CGI::checkbox( >- -name => "repeatable$i", >- -id => "repeatable$i", >- -checked => '', >- -value => 1, >- -label => '' >- ); >- $row_data{mandatory} = CGI::checkbox( >- -name => "mandatory$i", >- -id => "mandatory$i", >- -checked => '', >- -value => 1, >- -label => '' >- ); >- $row_data{isurl} = CGI::checkbox( >- -name => "isurl$i", >- -id => "isurl$i", >- -checked => '', >- -value => 1, >- -label => '' >- ); >+ $row_data{repeatable} = { >+ name => "repeatable$i", >+ id => "repeatable$i", >+ checked => '', >+ }; >+ $row_data{mandatory} = { >+ name => "mandatory$i", >+ id => "mandatory$i", >+ checked => '', >+ }; >+ $row_data{isurl} = { >+ name => "isurl$i", >+ id => "isurl$i", >+ checked => '', >+ }; > $row_data{value_builder} = { > id => "value_builder$i", > values => \@value_builder, >@@ -303,7 +291,7 @@ if ( $op eq 'add_form' ) { > values => \@authtypes, > default => $data->{'authtypecode'}, > }; >- $row_data{link} = CGI::escapeHTML( $data->{'link'} ); >+ $row_data{link} = $data->{'link'}; > $row_data{row} = $i; > push( @loop_data, \%row_data ); > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/auth_subfields_structure.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/auth_subfields_structure.tt >index 87b468b..2ec86f1 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/auth_subfields_structure.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/auth_subfields_structure.tt >@@ -62,9 +62,9 @@ function displayMoreConstraint(numlayer){ > <ul> > [% FOREACH loo IN loop %] > [% IF ( loo.new_subfield ) %] >- <li><a href="#sub[% loo.tagsubfield %]field" title="[% loo.liblibrarian %]">New</a></li> >+ <li><a href="#sub[% loo.tagsubfield %]field" title="[% loo.liblibrarian | html_entity %]">New</a></li> > [% ELSE %] >- <li><a href="#sub[% loo.tagsubfield %]field" title="[% loo.liblibrarian %]"> >+ <li><a href="#sub[% loo.tagsubfield %]field" title="[% loo.liblibrarian | html_entity %]"> > [% loo.tagsubfield %] > </a></li> > [% END %] >@@ -82,10 +82,26 @@ function displayMoreConstraint(numlayer){ > <input type="hidden" name="tagsubfield" value="[% loo.tagsubfield %]" /> > </li> > [% END %] >- <li><label for="repeatable[% loo.row %]">Repeatable: </label>[% loo.repeatable %] </li> >- <li><label for="mandatory[% loo.row %]">Mandatory: </label>[% loo.mandatory %] </li> >- <li><label for="liblibrarian[% loo.row %]">Text for librarian: </label><input id="liblibrarian[% loo.row %]" type="text" name="liblibrarian" value="[% loo.liblibrarian %]" size="40" maxlength="80" /></li> >- <li><label for="libopac[% loo.row %]">Text for OPAC: </label><input type="text" id="libopac[% loo.row %]" name="libopac" value="[% loo.libopac %]" size="40" maxlength="80" /></li> >+ <li><label for="repeatable[% loo.row %]">Repeatable:</label> >+ <label> >+ [% IF ( loo.repeatable.checked ) %] >+ <input type="checkbox" name="[% loo.repeatable.name %]" value="1" id="[% loo.repeatable.id %]" checked="checked" /> >+ [% ELSE %] >+ <input type="checkbox" name="[% loo.repeatable.name %]" value="1" id="[% loo.repeatable.id %]" /> >+ [% END %] >+ </label> >+ </li> >+ <li><label for="mandatory[% loo.row %]">Mandatory:</label> >+ <label> >+ [% IF ( loo.mandatory.checked ) %] >+ <input type="checkbox" name="[% loo.mandatory.name %]" value="1" id="[% loo.mandatory.id %]" checked="checked" /> >+ [% ELSE %] >+ <input type="checkbox" name="[% loo.mandatory.name %]" value="1" id="[% loo.mandatory.id %]" /> >+ [% END %] >+ </label> >+ </li> >+ <li><label for="liblibrarian[% loo.row %]">Text for librarian: </label><input id="liblibrarian[% loo.row %]" type="text" name="liblibrarian" value="[% loo.liblibrarian | html_entity %]" size="40" maxlength="80" /></li> >+ <li><label for="libopac[% loo.row %]">Text for OPAC: </label><input type="text" id="libopac[% loo.row %]" name="libopac" value="[% loo.libopac | html_entity %]" size="40" maxlength="80" /></li> > <li><label for="tab[% loo.row %]">Managed in tab: </label> > <select name="tab" size="1" id="[% loo.tab.id %]"> > [%- IF ( loo.tab.default == -1 ) -%] >@@ -130,7 +146,16 @@ function displayMoreConstraint(numlayer){ > <fieldset class="rows"> > <legend>Advanced constraints:</legend> > <ol> >- <li><label for="isurl[% loo.row %]">Is a URL:</label>[% loo.isurl %] (if checked, it means that the subfield is a URL and can be clicked)</li> >+ <li><label for="isurl[% loo.row %]">Is a URL:</label> >+ <label> >+ [% IF ( loo.isurl.checked ) %] >+ <input type="checkbox" name="[% loo.isurl.name %]" value="1" id="[% loo.isurl.id %]" checked="checked" /> >+ [% ELSE %] >+ <input type="checkbox" name="[% loo.isurl.name %]" value="1" id="[% loo.isurl.id %]" /> >+ [% END %] >+ </label> >+ <span>(if checked, it means that the subfield is a URL and can be clicked)</span> >+ </li> > <li> > <label for="defaultvalue[% loo.row %]">Default value:</label> > <input type="text" name="defaultvalue" id="defaultvalue[% loo.row %]" value="[% loo.defaultvalue %]" /> >@@ -264,9 +289,9 @@ function displayMoreConstraint(numlayer){ > <td>[% loo.tagsubfield %]</td> > <td> > [% IF ( loo.subfield_ignored ) %] >- <i>[% loo.liblibrarian %]</i> >+ <i>[% loo.liblibrarian | html_entity %]</i> > [% ELSE %] >- [% loo.liblibrarian %] >+ [% loo.liblibrarian | html_entity %] > [% END %] > </td> > <td> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_subfields_structure.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_subfields_structure.tt >index bfa8815..81197ab 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_subfields_structure.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_subfields_structure.tt >@@ -204,9 +204,9 @@ function populateHiddenCheckboxes(tab) { > <ul> > [% FOREACH loo IN loop %] > [% IF ( loo.new_subfield ) %] >- <li><a href="#sub[% loo.urisubfieldcode %]field" title="[% loo.liblibrarian %]">New</a></li> >+ <li><a href="#sub[% loo.urisubfieldcode %]field" title="[% loo.liblibrarian | html_entity %]">New</a></li> > [% ELSE %] >- <li><a href="#sub[% loo.urisubfieldcode %]field" title="[% loo.liblibrarian %]"> >+ <li><a href="#sub[% loo.urisubfieldcode %]field" title="[% loo.liblibrarian | html_entity %]"> > [% loo.subfieldcode %] > </a></li> > [% END %] >@@ -225,10 +225,27 @@ function populateHiddenCheckboxes(tab) { > [% ELSE %] > <li><label for="tagsubfield[% loo.row %]">Subfield code:</label> <input type="text" id="tagsubfield[% loo.row %]" name="tagsubfield" value="[% loo.subfieldcode %]" /></li> > [% END %] >- <li><label for="liblibrarian[% loo.row %]">Text for librarian: </label><input id="liblibrarian[% loo.row %]" type="text" name="liblibrarian" value="[% loo.liblibrarian %]" size="40" maxlength="80" /></li> >- <li><label for="libopac[% loo.row %]">Text for OPAC: </label><input type="text" id="libopac[% loo.row %]" name="libopac" value="[% loo.libopac %]" size="40" maxlength="80" /></li> >- <li><label for="repeatable[% loo.row %]">Repeatable: </label>[% loo.repeatable %]</li> >- <li><label for="mandatory[% loo.row %]">Mandatory: </label>[% loo.mandatory %]</li> >+ <li><label for="liblibrarian[% loo.row %]">Text for librarian: </label><input id="liblibrarian[% loo.row %]" type="text" name="liblibrarian" value="[% loo.liblibrarian | html_entity %]" size="40" maxlength="80" /></li> >+ <li><label for="libopac[% loo.row %]">Text for OPAC: </label><input type="text" id="libopac[% loo.row %]" name="libopac" value="[% loo.libopac | html_entity %]" size="40" maxlength="80" /></li> >+ <li><label for="repeatable[% loo.row %]">Repeatable:</label> >+ <label> >+ [% IF ( loo.repeatable.checked ) %] >+ <input type="checkbox" name="[% loo.repeatable.name %]" value="1" id="[% loo.repeatable.id %]" checked="checked" /> >+ [% ELSE %] >+ <input type="checkbox" name="[% loo.repeatable.name %]" value="1" id="[% loo.repeatable.id %]" /> >+ [% END %] >+ </label> >+ </li> >+ <li><label for="mandatory[% loo.row %]">Mandatory:</label> >+ <label> >+ [% IF ( loo.mandatory.checked ) %] >+ <input type="checkbox" name="[% loo.mandatory.name %]" value="1" id="[% loo.mandatory.id %]" checked="checked" /> >+ [% ELSE %] >+ <input type="checkbox" name="[% loo.mandatory.name %]" value="1" id="[% loo.mandatory.id %]" /> >+ [% END %] >+ </label> >+ </li> >+ > <li><label for="tab[% loo.row %]">Managed in tab: </label> > <select name="tab" tabindex="" size="1" id="[% loo.tab.id %]"> > [%- IF ( loo.tab.default == -1 ) -%] >@@ -262,7 +279,7 @@ function populateHiddenCheckboxes(tab) { > <ol><li><label for="defaultvalue[% loo.row %]">Default value:</label> > <input type="text" name="defaultvalue" id="defaultvalue[% loo.row %]" value="[% loo.defaultvalue %]" /></li> > <li><label for="maxlength[% loo.row %]">Max length:</label><input type="text" id="maxlength[% loo.row %]" name="maxlength" value="[% loo.maxlength %]" size="4" /> (see online help)</li> >- <li><input type="hidden" id="hidden-[% loo.row %]" name="hidden" value="[% loo.hidden %]" /> >+ <li><input type="hidden" id="hidden-[% loo.row %]" name="hidden" value="[% loo.hidden | html_entity %]" /> > <label for="hidden[% loo.row %]" style="float: none;">Visibility: </label> > <input type="checkbox" id="hidden_opac_[% loo.row %]" class="inclusive_[% loo.row %]" name="hidden_opac_[% loo.row %]"/> > <label for="hidden_opac_[% loo.row %]" style="float: none;">OPAC</label> >@@ -275,8 +292,17 @@ function populateHiddenCheckboxes(tab) { > <input type="checkbox" id="hidden_flagged_[% loo.row %]" name="flagged_[% loo.row %]"/> > <label for="hidden_flagged_[% loo.row %]" style="float: none;">Flagged</label> > </li> >- <li><label for="isurl[% loo.row %]">Is a URL:</label>[% loo.isurl %] (if checked, it means that the subfield is a URL and can be clicked)</li> >- <li><label for="link[% loo.row %]">Link:</label><input type="text" id="link[% loo.row %]" name="link" value="[% loo.link %]" size="10" maxlength="80" /> (e.g., Title or Local-Number) <span class="error"><em>NOTE: If you change this value you must ask your administrator to run misc/batchRebuildBiblioTables.pl.</em></span></li> >+ <li><label for="isurl[% loo.row %]">Is a URL:</label> >+ <label> >+ [% IF ( loo.isurl.checked ) %] >+ <input type="checkbox" name="[% loo.isurl.name %]" value="1" id="[% loo.isurl.id %]" checked="checked" /> >+ [% ELSE %] >+ <input type="checkbox" name="[% loo.isurl.name %]" value="1" id="[% loo.isurl.id %]" /> >+ [% END %] >+ </label> >+ <span>(if checked, it means that the subfield is a URL and can be clicked)</span> >+ </li> >+ <li><label for="link[% loo.row %]">Link:</label><input type="text" id="link[% loo.row %]" name="link" value="[% loo.link | html_entity %]" size="10" maxlength="80" /> (e.g., Title or Local-Number) <span class="error"><em>NOTE: If you change this value you must ask your administrator to run misc/batchRebuildBiblioTables.pl.</em></span></li> > <li> > <label for="kohafield[% loo.row %]">Koha link:</label> > <select name="kohafield" id="[% loo.kohafield.id %]" size="1"> >@@ -395,9 +421,9 @@ function populateHiddenCheckboxes(tab) { > <td><a href="/cgi-bin/koha/admin/marc_subfields_structure.pl?op=add_form&tagfield=[% loo.tagfield %]&frameworkcode=[% frameworkcode %]#sub[% loo.tagsubfield %]field">[% loo.tagsubfield %]</a></td> > <td> > [% IF ( loo.subfield_ignored ) %] >- <i>[% loo.liblibrarian %]</i> >+ <i>[% loo.liblibrarian | html_entity %]</i> > [% ELSE %] >- [% loo.liblibrarian %] >+ [% loo.liblibrarian | html_entity %] > [% END %] > </td> > <td> >@@ -408,13 +434,13 @@ function populateHiddenCheckboxes(tab) { > [% IF ( loo.kohafield ) %] | Koha field: [% loo.kohafield %], [% END %] > [% IF ( loo.repeatable ) %]Repeatable, [% ELSE %]Not repeatable,[% END %] > [% IF ( loo.mandatory ) %]Mandatory, [% ELSE %]Not mandatory,[% END %] >- [% IF ( loo.seealso ) %] | See Also: [% loo.seealso %],[% END %] >+ [% IF ( loo.seealso ) %] | See Also: [% loo.seealso | html_entity %],[% END %] > [% IF ( loo.hidden ) %]hidden,[% END %] > [% IF ( loo.isurl ) %]is a URL,[% END %] > [% IF ( loo.authorised_value ) %] | Auth value:[% loo.authorised_value %],[% END %] > [% IF ( loo.authtypecode ) %] | Authority:[% loo.authtypecode %],[% END %] > [% IF ( loo.value_builder ) %] | Plugin:[% loo.value_builder %],[% END %] >- [% IF ( loo.link ) %] | Link:[% loo.link %],[% END %] >+ [% IF ( loo.link ) %] | Link: [% loo.link | html_entity %],[% END %] > [% END %] > </td> > <td><a href="/cgi-bin/koha/admin/marc_subfields_structure.pl?op=add_form&tagfield=[% loo.tagfield %]&frameworkcode=[% frameworkcode %]#sub[% loo.tagsubfield %]field">Edit</a></td> >-- >1.9.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14308
: 39739