As far as I can tell, the C4::Scrubber profiles "staff" and "tag" are not actually used in Koha. "tag" is actually the same as "default", and opac-tags.pl just uses "default". There's no point to "tag". Now "staff" is actually dangerous, because it doesn't do any scrubbing. It allows all elements and all attributes. We never want to use it.
Created attachment 183021 [details] [review] Bug 40087: Remove "tag" and "staff" scrubber profiles This patch removes unused "tag" and "staff" scrubber profiles. Test plan: 0. Apply the patch 1. prove t/Koha/Plugins/HtmlScrubber.t 2. prove t/Scrubber.t 3. Grep around the code for C4::Scrubber and make sure "tag" and "staff" aren't used as arguments for C4::Scrubber
prove t/Scrubber.t fails for me (both tests pass before the patch): t/Scrubber.t .. 1/32 New called with unrecognized type 'tag' at t/Scrubber.t line 60. # Looks like your test exited with 255 just after 18. t/Scrubber.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 14/32 subtests Test Summary Report ------------------- t/Scrubber.t (Wstat: 65280 (exited 255) Tests: 18 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 32 tests but ran 18. Files=1, Tests=18, 1 wallclock secs ( 0.01 usr 0.01 sys + 0.22 cusr 0.05 csys = 0.29 CPU) Result: FAIL Not sure what I'm looking for, for step 3 of the test plan.
(In reply to David Nind from comment #2) > prove t/Scrubber.t fails for me (both tests pass before the patch): > > t/Scrubber.t .. 1/32 New called with unrecognized type 'tag' at t/Scrubber.t > line 60. > # Looks like your test exited with 255 just after 18. > t/Scrubber.t .. Dubious, test returned 255 (wstat 65280, 0xff00) > Failed 14/32 subtests > > Test Summary Report > ------------------- > t/Scrubber.t (Wstat: 65280 (exited 255) Tests: 18 Failed: 0) > Non-zero exit status: 255 > Parse errors: Bad plan. You planned 32 tests but ran 18. > Files=1, Tests=18, 1 wallclock secs ( 0.01 usr 0.01 sys + 0.22 cusr 0.05 > csys = 0.29 CPU) > Result: FAIL > > Not sure what I'm looking for, for step 3 of the test plan. Thanks, David. I actually ran it with "perl t/Scrubber.t" originally and it looks like I missed the failure because of the lack of color-coding haha. I'll get that fixed.
Created attachment 183032 [details] [review] Bug 40087: Fix t/Scrubber.t unit test Remove "tag" and "staff" and add the missing "note"
Round 2...
Created attachment 183033 [details] [review] Bug 40087: Remove "tag" and "staff" scrubber profiles This patch removes unused "tag" and "staff" scrubber profiles. Test plan: 0. Apply the patch 1. prove t/Koha/Plugins/HtmlScrubber.t 2. prove t/Scrubber.t 3. Grep around the code for C4::Scrubber and make sure "tag" and "staff" aren't used as arguments for C4::Scrubber Signed-off-by: David Nind <david@davidnind.com>
Created attachment 183034 [details] [review] Bug 40087: Fix t/Scrubber.t unit test Remove "tag" and "staff" and add the missing "note" Signed-off-by: David Nind <david@davidnind.com>
(In reply to David Cook from comment #3) > I'll get that fixed. Thanks David, signed off. For step 3, not sure exactly what to look for. With a git grep "C4::Scrubber", I can't see any mention of tag and staff.
Created attachment 183036 [details] [review] Bug 40087: Remove "tag" and "staff" scrubber profiles This patch removes unused "tag" and "staff" scrubber profiles. Test plan: 0. Apply the patch 1. prove t/Koha/Plugins/HtmlScrubber.t 2. prove t/Scrubber.t 3. Grep around the code for C4::Scrubber and make sure "tag" and "staff" aren't used as arguments for C4::Scrubber Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 183037 [details] [review] Bug 40087: Fix t/Scrubber.t unit test Remove "tag" and "staff" and add the missing "note" Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Nice work everyone! Pushed to main for 25.11
Nice work everyone! Pushed to 25.05.x for 25.05.03