Bugzilla – Attachment 27674 Details for
Bug 12091
HTML from auth_subfields_structure.tt doesn't pass firefox's html validator
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12091 - Rebased, but untested still.
Bug-12091---Rebased-but-untested-still.patch (text/plain), 15.59 KB, created by
Mark Tompsett
on 2014-04-27 04:27:24 UTC
(
hide
)
Description:
Bug 12091 - Rebased, but untested still.
Filename:
MIME Type:
Creator:
Mark Tompsett
Created:
2014-04-27 04:27:24 UTC
Size:
15.59 KB
patch
obsolete
>From 51db9340638af99ab7f9159540006e877aadb878 Mon Sep 17 00:00:00 2001 >From: Mark Tompsett <mtompset@hotmail.com> >Date: Sun, 27 Apr 2014 00:19:09 -0400 >Subject: [PATCH] Bug 12091 - Rebased, but untested still. > >This cleans up the HTML in auth_subfields_structure.pl by: >- removing some more CGI::scrolling_lists. >- fixing some attributes >- removing some attributes >- and using a handy-dandy validation plug-in that Owen pointed > me at the other day. >--- > admin/auth_subfields_structure.pl | 152 +++++++------------- > .../en/modules/admin/auth_subfields_structure.tt | 77 +++++++--- > 2 files changed, 112 insertions(+), 117 deletions(-) > >diff --git a/admin/auth_subfields_structure.pl b/admin/auth_subfields_structure.pl >index 321e54c..68961db 100755 >--- a/admin/auth_subfields_structure.pl >+++ b/admin/auth_subfields_structure.pl >@@ -23,6 +23,7 @@ use C4::Output; > use C4::Auth; > use CGI; > use C4::Context; >+use C4::Koha; > > > sub string_search { >@@ -91,25 +92,14 @@ if ($op eq 'add_form') { > push @kohafields, "auth_header.".$field; > } > >- # build authorised value list >- $sth2 = $dbh->prepare("select distinct category from authorised_values"); >- $sth2->execute; >- my @authorised_values; >- push @authorised_values,""; >- while ((my $category) = $sth2->fetchrow_array) { >- push @authorised_values, $category; >- } >- push (@authorised_values,"branches"); >- push (@authorised_values,"itemtypes"); >+ # build authorised value list >+ my $authorised_values = C4::Koha::GetAuthorisedValueCategories; >+ unshift @$authorised_values,""; >+ push (@$authorised_values,"branches"); >+ push (@$authorised_values,"itemtypes"); > > # build thesaurus categories list >- $sth2 = $dbh->prepare("select authtypecode from auth_types"); >- $sth2->execute; >- my @authtypes; >- push @authtypes, ""; >- while ( ( my $authtypecode ) = $sth2->fetchrow_array ) { >- push @authtypes, $authtypecode; >- } >+ my $authtypes = getauthtypes; > > # build value_builder list > my @value_builder=(''); >@@ -180,53 +170,37 @@ if ($op eq 'add_form') { > -size=>1, > -multiple=>0, > ); >- $row_data{authorised_value} = CGI::scrolling_list(-name=>'authorised_value', >- -id=>"authorised_value$i", >- -values=> \@authorised_values, >- -default=>$data->{'authorised_value'}, >- -size=>1, >- -tabindex=>'', >- -multiple=>0, >- ); >- $row_data{frameworkcode} = CGI::scrolling_list(-name=>'frameworkcode', >- -id=>"frameworkcode$i", >- -values=> \@authtypes, >- -default=>$data->{'frameworkcode'}, >- -size=>1, >- -tabindex=>'', >- -multiple=>0, >- ); >- $row_data{value_builder} = CGI::scrolling_list(-name=>'value_builder', >- -id=>"value_builder$i", >- -values=> \@value_builder, >- -default=>$data->{'value_builder'}, >- -size=>1, >- -tabindex=>'', >- -multiple=>0, >- ); >- >- $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{authorised_value} = { >+ id => "authorised_value$i", >+ codes => $authorised_values, >+ default => $data->{'authorised_value'}, >+ }; >+ $row_data{frameworkcode} = { >+ id => "frameworkcode$i", >+ codes => $authtypes, >+ default => $data->{'frameworkcode'}, >+ }; >+ $row_data{value_builder} = { >+ id => "value_builder$i", >+ codes => \@value_builder, >+ default => $data->{'value_builder'}, >+ }; >+ $row_data{repeatable} = "<input id=\"repeatable$i\" type=\"checkbox\" " . >+ ( $data->{'repeatable'} ? 'checked="checked" ' : '') . >+ "name=\"repeatable$i\">"; >+ $row_data{mandatory} = "<input id=\"mandatory$i\" type=\"checkbox\" " . >+ ( $data->{'mandatory'} ? 'checked="checked" ' : '' ) . >+ "name=\"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{isurl} = "<input id=\"isurl$i\" type=\"checkbox\" " . >+ ( $data->{'isurl'} ? 'checked="checked" ' : '' ) . >+ "name=\"isurl$i\">"; > $row_data{row} = $i; > push(@loop_data, \%row_data); > $i++; > } > # add more_subfields empty lines for add if needed >- for (my $i=1;$i<=$more_subfields;$i++) { >+ for (my $j=1;$j<=$more_subfields;$j++) { > my %row_data; # get a fresh hash for the row data > $row_data{'new_subfield'} = 1; > $row_data{tab} = CGI::scrolling_list(-name=>'tab', >@@ -248,7 +222,6 @@ if ($op eq 'add_form') { > }, > -default=>"", > -size=>1, >- -tabindex=>'', > -multiple=>0, > ); > $row_data{ohidden} = CGI::scrolling_list(-name=>'ohidden', >@@ -287,30 +260,14 @@ if ($op eq 'add_form') { > # -multiple=>0, > # ); > $row_data{tagsubfieldinput} = "<input type=\"text\" name=\"tagsubfield\" value=\"".$data->{'tagsubfield'}."\" size=\"1\" id=\"tagsubfield\" maxlength=\"1\" />"; >- $row_data{tagsubfieldinput} = >- "<label><input type=\"text\" name=\"tagsubfield\" value=\"" >- . $data->{'tagsubfield'} >- . "\" size=\"1\" id=\"tagsubfield\" maxlength=\"1\" /></label>"; > $row_data{tagsubfield} = $data->{'tagsubfield'}; > $row_data{liblibrarian} = ""; > $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} = "<input id=\"repeatable$i\" type=\"checkbox\" name=\"repeatable$i\">"; >+ $row_data{mandatory} = "<input id=\"mandatory$i\" type=\"checkbox\" name=\"mandatory$i\">"; >+ $row_data{isurl} = "<input id=\"isurl$i\" type=\"checkbox\" name=\"isurl$i\">"; > $row_data{kohafield}= CGI::scrolling_list( -name=>'kohafield', > -id => "kohafield$i", > -values=> \@kohafields, >@@ -318,30 +275,23 @@ if ($op eq 'add_form') { > -size=>1, > -multiple=>0, > ); >- $row_data{frameworkcode} = CGI::scrolling_list(-name=>'frameworkcode', >- -id=>'frameworkcode', >- -values=> \@authtypes, >- -default=>$data->{'frameworkcode'}, >- -size=>1, >- -tabindex=>'', >- -multiple=>0, >- ); >- $row_data{authorised_value} = CGI::scrolling_list(-name=>'authorised_value', >- -id => 'authorised_value', >- -values=> \@authorised_values, >- -size=>1, >- -tabindex=>'', >- -multiple=>0, >- ); >- $row_data{value_builder} = CGI::scrolling_list(-name=>'value_builder', >- -id=>'value_builder', >- -values=> \@value_builder, >- -default=>$data->{'value_builder'}, >- -size=>1, >- -tabindex=>'', >- -multiple=>0, >- ); >+ $row_data{frameworkcode} = { >+ id => 'frameworkcode', >+ codes => $authtypes, >+ default => $data->{'frameworkcode'}, >+ }; >+ $row_data{authorised_value} = { >+ id => 'authorised_value', >+ codes => $authorised_values, >+ default => $data->{'authorised_values'}, >+ }; >+ $row_data{value_builder} = { >+ id => 'value_builder', >+ codes => \@value_builder, >+ default => $data->{'value_builder'}, >+ }; > $row_data{row} = $i; >+ ++$i; > push(@loop_data, \%row_data); > } > $template->param('use_heading_flags_p' => 1); >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 94d53bf..7c31113 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 >@@ -76,20 +76,17 @@ function displayMoreConstraint(numlayer){ > [% FOREACH loo IN loop %] > <div id="sub[% loo.tagsubfield %]field"> > <fieldset class="rows"><ol> >- >- [% IF ( loo.new_subfield ) %] >- <li><label for="tagsubfieldinput[% loo.row %]">Subfield code: </label>[% loo.tagsubfieldinput %] </li> >- [% ELSE %] >- <li> >- <input type="hidden" name="tagsubfield" value="[% loo.tagsubfield %]" /> >- </li> >- [% END %] >+ [% IF ( loo.new_subfield ) %] >+ <li><label for="tagsubfield">Subfield code: </label>[% loo.tagsubfieldinput %] </li> >+ [% ELSE %] >+ <li><input type="hidden" name="tagsubfield" value="[% loo.tagsubfield %]" /> >+ [% 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="liblibrarian[% loo.row %]">Text for librarian: </label><input id="liblibrarian[% loo.row %]" type="text" name="liblibrarian[% loo.row %]" 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[% loo.row %]" value="[% loo.libopac %]" size="40" maxlength="80" /></li> > <li><label for="tab[% loo.row %]">Managed in tab: </label> >- <select name="tab" tabindex="" size="1" id="[% loo.tab.id %]"> >+ <select name="tab[% loo.row %]" size="1" id="tab[% loo.row %]"> > [%- IF ( loo.tab.default == -1 ) -%] > <option value="-1" selected="selected">ignore</option> > [%- ELSE -%] >@@ -110,7 +107,7 @@ function displayMoreConstraint(numlayer){ > <legend>Display</legend> > <ol> > <li><label for="ohidden[% loo.row %]">Select to display or not:</label> >- <select name="ohidden" size="1" id="[% loo.ohidden.id %]"> >+ <select name="ohidden[% loo.row %]" size="1" id="ohidden[% loo.row %]"> > [%- IF ( loo.ohidden.default == 0 ) -%] > <option value= "0" selected="selected">Show all</option> > <option value="-5">Hide all</option> >@@ -135,7 +132,7 @@ function displayMoreConstraint(numlayer){ > <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="defaultvalue[% loo.row %]">Default value:</label> >- <input type="text" name="defaultvalue" id="defaultvalue[% loo.row %]" value="[% loo.defaultvalue %]" /> >+ <input type="text" name="defaultvalue[% loo.row %]" id="defaultvalue[% loo.row %]" value="[% loo.defaultvalue %]" /> > </li> > </ol> > </fieldset> >@@ -143,9 +140,57 @@ function displayMoreConstraint(numlayer){ > <li> > <fieldset><legend>Help input</legend> > <ol> >- <li><label for="authorised_value[% loo.row %]">Authorized value:</label> [% loo.authorised_value %]</li> >- <li><label for="frameworkcode[% loo.row %]">Thesaurus:</label> [% loo.frameworkcode %]</li> >- <li><label for="value_builder[% loo.row %]">Plugin:</label> [% loo.value_builder %]</li> >+ <li> >+<label for="authorised_value[% loo.row %]">Authorized value:</label> >+<select name="authorised_value[% loo.row %]" size="1" id="authorised_value[% loo.row %]"> >+[%- FOREACH t IN loo.authorised_value.codes -%] >+ [%- IF loo.authorised_value.default == t && t == "" %] >+ <!-- this case for a space is necessary to validate HTML nicely. --> >+ <option selected="selected" value="[% t %]"> </option> >+ [%- ELSIF loo.frameworkcode.default == t %] >+ <option selected="selected" value="[% t %]">[% t %]</option> >+ [%- ELSIF t == "" %] >+ <option value=""> </option> >+ [%- ELSE %] >+ <option value="[% t %]">[% t %]</option> >+ [%- END %] >+[%- END -%] >+</select> >+ </li> >+ <li> >+<label for="frameworkcode[% loo.row %]">Thesaurus:</label> >+<select name="frameworkcode[% loo.row %]" size="1" id="frameworkcode[% loo.row %]"> >+[%- FOREACH t IN loo.frameworkcode.codes -%] >+ [%- IF loo.frameworkcode.default == t.value.authtypecode && t.value.authtypecode == "" %] >+ <!-- this case for a space is necessary to validate HTML nicely. --> >+ <option selected="selected" value="[% t.value.authtypecode %]"> </option> >+ [%- ELSIF loo.frameworkcode.default == t.value.authtypecode %] >+ <option selected="selected" value="[% t.value.authtypecode %]">[% t.value.authtypecode %]</option> >+ [%- ELSIF t.value.authtypecode == "" %] >+ <option value=""> </option> >+ [%- ELSE %] >+ <option value="[% t.value.authtypecode %]">[% t.value.authtypecode %]</option> >+ [%- END %] >+[%- END -%] >+</select> >+ </li> >+ <li> >+<label for="value_builder[% loo.row %]">Plugin:</label> >+<select name="value_builder[% loo.row %]" size="1" id="value_builder[% loo.row %]"> >+[%- FOREACH t IN loo.value_builder.codes -%] >+ [%- IF loo.value_builder.default == t && t == "" %] >+ <!-- this case for a space is necessary to validate HTML nicely. --> >+ <option selected="selected" value="[% t %]"> </option> >+ [%- ELSIF loo.value_builder.default == t %] >+ <option selected="selected" value="[% t %]">[% t %]</option> >+ [%- ELSIF t == "" %] >+ <option value=""> </option> >+ [%- ELSE %] >+ <option value="[% t %]">[% t %]</option> >+ [%- END %] >+[%- END -%] >+</select> >+ </li> > </ol> > </fieldset> > </li> >-- >1.7.9.5
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 12091
:
27152
|
27674
|
30767
|
30768
|
30773
|
30774
|
30778
|
30779
|
30826