[Bug 12091] New: Ugly HTML for editing authority MARC subfields
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 Bug ID: 12091 Summary: Ugly HTML for editing authority MARC subfields Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Staff Client Assignee: koha-bugs@lists.koha-community.org Reporter: mtompset@hotmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com After installing the 0.9.5.8 HTML Validator plugin for Firefox, as suggested by Owen, I noticed a lot of errors for /cgi-bin/koha/admin/auth_subfields_structure.pl while testing another bug. The goal of this patch is to bring the validations errors to as close to 0 as possible. I think my untested patch gets 2 warnings. I have to confirm that the screen still functions properly. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 --- Comment #1 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 27152 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27152&action=edit Bug 12091 - Ugly HTML corrects for /cgi-bin/koha/admin/auth_subfields_structure.pl Untested. I know it cleans up the HTML, but with the renaming of things and the like, it needs to be thoroughly tested still. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 --- Comment #2 from M. Tompsett <mtompset@hotmail.com> --- Working on a rebase because of bug 11508. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27152|0 |1 is obsolete| | --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 27674 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27674&action=edit 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=11508 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |mtompset@hotmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bgkriegel@gmail.com --- Comment #4 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Mark, no problem to test but... 1. Test plan is missing 2. I think that we must try (fervently) not to put HTML in scripts So, this kind of construct $row_data{repeatable} = "<input id=\"repeatable$i\" type=\"checkbox\" " . ( $data->{'repeatable'} ? 'checked="checked" ' : '') . "name=\"repeatable$i\"> it's calling for a FaliedQA :( Perhaps can be replaced by (in script) $row_data{repeatable} = { id => repeatable$i, name => repeatable$i, repeatable => $data->{'repeatable'}, } (in tt) [% IF ( ...row.repeatable ) %] [% IF ( ...row.repeatable.repeatable ) %] <input id="[%- ...row.repeatable.id -%]" type="checkbox" name="[%- ...row.repeatable.name -%]" checked="checked"> [% ELSE %] <input id="[%- ...row.repeatable.id -%]" type="checkbox" name="[%- ...row.repeatable.name -%]"> [% END %] [% END %] or something similar 3. Bug status, it's ready to be signed? Then change that :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 --- Comment #5 from M. Tompsett <mtompset@hotmail.com> --- That's why the name of the patch is untested and the status is still new. And yes, after getting it validating HTML-wise, I realized that HTML in scripts is bad. As for a test plan, trust me, it will be lengthy and thorough when it is finished being tested so as to make testing very simple. I'm just using bugzilla as my offsite backup, so I can do things like "git reset --hard origin/master" and not worry about losing the patch. :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- Well, another bug that Bernardo Gonzalez Kriegel cleaned up (bug 12614) made me have to rework this. I'm still concerned about the duplicate ID stuff, and if the data is actually being saved properly. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=12091 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27674|0 |1 is obsolete| | --- Comment #7 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 30767 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30767&action=edit Bug 12091 - Part that should work This cleans up the HTML in auth_subfields_structure.pl and auth_subfields_structure.tt by: - fixing some attributes - removing some attributes - cleaning up list building code - and using a handy-dandy validation plug-in that Owen pointed me at the other day. TEST PLAN --------- 1) Install HTML Validator Plugin (Html Validator 0.9.5.8 Firefox addon) 2) Log in to staff client 3) Koha administration 4) Authority types (on the right, 5 down from Catalog header) 5) Click 'MARC structure' of any auth type 6) Click 'subfields' (I believe for any Tag >= 010) 7) Click 'Edit subfields' 8) Right click and select 'View Page Source' -- Lots of HTML validation errors 9) View the 'Authorized value:' and 'Thesaurus:' drop down lists. -- Patch cleans up code, so lists should remain unchanged after applying patch 10) Apply the patch 11) Refresh the page (make sure it isn't cached!) 12) Right click and select 'View Page Source' -- It should be down to 10 errors / 11 warnings. NOTE: I think this means there is a code problem with actually editing authorities properly! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30767|0 |1 is obsolete| | --- Comment #8 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 30768 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30768&action=edit Bug 12091 - Part that should work This cleans up the HTML in auth_subfields_structure.pl and auth_subfields_structure.tt by: - fixing some attributes - removing some attributes - and using a handy-dandy validation plug-in that Owen pointed me at the other day. TEST PLAN --------- 1) Install HTML Validator Plugin (Html Validator 0.9.5.8 Firefox addon) 2) Log in to staff client 3) Koha administration 4) Authority types (on the right, 5 down from Catalog header) 5) Click 'MARC structure' of any auth type 6) Click 'subfields' (I believe for any Tag >= 010) 7) Click 'Edit subfields' 8) Right click and select 'View Page Source' -- Lots of HTML validation errors 9) View the 'Authorized value:' and 'Thesaurus:' drop down lists. -- Patch cleans up code, so lists should remain unchanged after applying patch 10) Apply the patch 11) Refresh the page (make sure it isn't cached!) 12) Right click and select 'View Page Source' -- It should be down to 10 errors / 11 warnings. NOTE: I think this means there is a code problem with actually editing authorities properly! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=12614 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Summary|Ugly HTML for editing |HTML from |authority MARC subfields |auth_subfields_structure.tt | |doesn't pass firefox's html | |validator -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30768|0 |1 is obsolete| | --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 30773 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30773&action=edit Bug 12091 - Part that should work This cleans up the HTML in auth_subfields_structure.pl and auth_subfields_structure.tt by: - fixing some attributes - removing some attributes - and using a handy-dandy validation plug-in that Owen pointed me at the other day. TEST PLAN --------- 1) Install HTML Validator Plugin (Html Validator 0.9.5.8 Firefox addon) 2) Log in to staff client 3) Koha administration 4) Authority types (on the right, 5 down from Catalog header) 5) Click 'MARC structure' of any auth type 6) Click 'subfields' (I believe for any Tag >= 010) 7) Click 'Edit subfields' 8) Right click and select 'View Page Source' -- Lots of HTML validation errors 9) View the 'Authorized value:' and 'Thesaurus:' drop down lists. -- Patch cleans up code, so lists should remain unchanged after applying patch 10) Apply the patch 11) Refresh the page (make sure it isn't cached!) 12) Right click and select 'View Page Source' -- It should be down to 10 errors / 11 warnings. NOTE: I think this means there is a code problem with actually editing authorities properly! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30773|0 |1 is obsolete| | --- Comment #10 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 30774 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30774&action=edit Bug 12091 - Part that should work This cleans up the HTML in auth_subfields_structure.pl and auth_subfields_structure.tt by: - fixing some attributes - removing some attributes - and using a handy-dandy validation plug-in that Owen pointed me at the other day. TEST PLAN --------- 1) Install HTML Validator Plugin (Html Validator 0.9.5.8 Firefox addon) 2) Log in to staff client 3) Koha administration 4) Authority types (on the right, 5 down from Catalog header) 5) Click 'MARC structure' of any auth type 6) Click 'subfields' (I believe for any Tag >= 010) 7) Click 'Edit subfields' 8) Right click and select 'View Page Source' -- Lots of HTML validation errors 9) View the 'Authorized value:' and 'Thesaurus:' drop down lists. -- Patch cleans up code, so lists should remain unchanged after applying patch 10) Apply the patch 11) Refresh the page (make sure it isn't cached!) 12) Right click and select 'View Page Source' -- It should be down to 10 errors / 11 warnings. NOTE: I think this means there is a code problem with actually editing authorities properly! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 --- Comment #11 from M. Tompsett <mtompset@hotmail.com> --- Maybe 11 errors / 11 warnings, lot less than before. :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30774|0 |1 is obsolete| | --- Comment #12 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 30778 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30778&action=edit [SIGNED-OFF] Bug 12091: HTML from auth_subfields_structure.tt doesn't pass firefox's html validator This cleans up the HTML in auth_subfields_structure.pl and auth_subfields_structure.tt by: - fixing some attributes - removing some attributes - and using a handy-dandy validation plug-in that Owen pointed me at the other day. TEST PLAN --------- 1) Install HTML Validator Plugin (Html Validator 0.9.5.8 Firefox addon) 2) Log in to staff client 3) Koha administration 4) Authority types (on the right, 5 down from Catalog header) 5) Click 'MARC structure' of any auth type 6) Click 'subfields' (I believe for any Tag >= 010) 7) Click 'Edit subfields' 8) Right click and select 'View Page Source' -- Lots of HTML validation errors 9) View the 'Authorized value:' and 'Thesaurus:' drop down lists. -- Patch cleans up code, so lists should remain unchanged after applying patch 10) Apply the patch 11) Refresh the page (make sure it isn't cached!) 12) Right click and select 'View Page Source' -- It should be down to 10 errors / 11 warnings. NOTE: I think this means there is a code problem with actually editing authorities properly! Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Work as described, much less validation errors, no koha-qa errors -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 --- Comment #13 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 30779 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30779&action=edit Bug 12091: Followup to fix remaining validation errors This patch fix all remaining errors. There was a for clause to add multiple new subfields, perhaps old code because now we can only add one subfield at a time, in that block id of html elements where numbered again from '1', and some elements lack proper id numbering, and one of them has incorrect label. I notice this on another patch but left things as they were trying not to broke anything. To test 1. Follow test plan of previous patch, there must be no errors, 1 warning 2. Add a new subfield, check subfield is added without problems -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 --- Comment #14 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- (In reply to M. Tompsett from comment #11)
Maybe 11 errors / 11 warnings, lot less than before. :)
Mark, I signed your patch and added a followup, I you can please test it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 --- Comment #15 from M. Tompsett <mtompset@hotmail.com> --- I recall fixing that ties into saving problems. It was a big unraveling mess. I will confirm and test later. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30779|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #16 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- (In reply to M. Tompsett from comment #15)
I recall fixing that ties into saving problems. It was a big unraveling mess. I will confirm and test later.
Ok, forget about it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 30826 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30826&action=edit [PASSED QA] Bug 12091: HTML from auth_subfields_structure.tt doesn't pass firefox's html validator This cleans up the HTML in auth_subfields_structure.pl and auth_subfields_structure.tt by: - fixing some attributes - removing some attributes - and using a handy-dandy validation plug-in that Owen pointed me at the other day. TEST PLAN --------- 1) Install HTML Validator Plugin (Html Validator 0.9.5.8 Firefox addon) 2) Log in to staff client 3) Koha administration 4) Authority types (on the right, 5 down from Catalog header) 5) Click 'MARC structure' of any auth type 6) Click 'subfields' (I believe for any Tag >= 010) 7) Click 'Edit subfields' 8) Right click and select 'View Page Source' -- Lots of HTML validation errors 9) View the 'Authorized value:' and 'Thesaurus:' drop down lists. -- Patch cleans up code, so lists should remain unchanged after applying patch 10) Apply the patch 11) Refresh the page (make sure it isn't cached!) 12) Right click and select 'View Page Source' -- It should be down to 10 errors / 11 warnings. NOTE: I think this means there is a code problem with actually editing authorities properly! Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Work as described, much less validation errors, no koha-qa errors Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> CÃleans up code, also removes SQL in favor of using a method. No regressions found. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #30778|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12091 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Mark! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org