Should not we allow span, div, etc. as well as existing "br b i em big small strong"?
Created attachment 163192 [details] [review] Bug 36328: Add p span div to Scrubber
Created attachment 163193 [details] [review] Bug 36328: Add test
I think it probably makes sense to have a few different profiles. For the "comment" scrubber, we probably want to keep it fairly minimal I think. Whereas "borrowernotes" and "opacnote" could have more elaborate structures. Bug 31934 has a profile with a fairly comprehensive list (although I wouldn't use it for "borrowernotes" or "opacnote"). I think an argument could be made for also including headings, table elements, fieldset/legend, ul/ol/li.
We could really do with getting bug 31934 moving again I think..
Created attachment 163497 [details] [review] Bug 36328: Add a separate 'note' profile
Would this patch work for you? Here I am trying to tackle down bug 19613.
(In reply to Jonathan Druart from comment #6) > Would this patch work for you? Here I am trying to tackle down bug 19613. Looks good. I'll look at signing this off pronto...
Created attachment 163582 [details] [review] Bug 36328: Add p span div to Scrubber Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 163583 [details] [review] Bug 36328: Add test Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 163584 [details] [review] Bug 36328: Add a separate 'note' profile Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 164206 [details] [review] Bug 36328: Add p span div to Scrubber Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 164207 [details] [review] Bug 36328: Add test Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 164208 [details] [review] Bug 36328: Add a separate 'note' profile Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 164209 [details] [review] Bug 36328: (QA follow-up) Expand tests and reorder elements to clarify differences Also tidy Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed for 24.05! Well done everyone, thank you!
Applied to 22.05.x
In hindsight, perhaps we should've allowed "id" and "class" attributes through for the "note" profile to allow people to easily add styling via CSS...
(In reply to David Cook from comment #17) > In hindsight, perhaps we should've allowed "id" and "class" attributes > through for the "note" profile to allow people to easily add styling via > CSS... Yes - new bug?
Comment on attachment 164208 [details] [review] Bug 36328: Add a separate 'note' profile Review of attachment 164208 [details] [review]: ----------------------------------------------------------------- ::: C4/Scrubber.pm @@ +50,4 @@ > rules => exists $settings->{rules} ? $settings->{rules} : [], > default => exists $settings->{default} ? $settings->{default} : [ 0 => { '*' => 0 } ], > comment => exists $settings->{comment} ? $settings->{comment} : 0, > + note => exists $settings->{note} ? $settings->{note} : 0, This line actually looks like it doesn't do anything. I'll open a bug to remove it as well.
(In reply to Katrin Fischer from comment #18) > (In reply to David Cook from comment #17) > > In hindsight, perhaps we should've allowed "id" and "class" attributes > > through for the "note" profile to allow people to easily add styling via > > CSS... > > Yes - new bug? Bug 38498