From 503eb018a556f538ab8b34b3d5b34dd5238580df Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Fri, 3 Feb 2023 12:00:39 +0100 Subject: [PATCH] Bug 30975: Fix framework plugins not working on cloned fields/subfields Since bug 29155 (Upgrade jquery version to 3.6.0) framework plugins do not work on a cloned field/subfield. That is because we rely on jQuery's `.data('events')` to clone event handlers to the cloned elements, and that was removed in jQuery 1.8.0 (I did not confirm but it's possible it continued to work after that thanks to jQuery Migrate) It is apparently still possible to access these event handlers by using `jQuery._data(element, "events")` but here's what jQuery 1.8.0 release notes says about it: "this is not a supported public interface; the actual data structures may change incompatibly from version to version." So we should not rely on it. Instead, what this patch does is use event delegation [1]. Events are bound to a parent container, so when elements are added dynamically inside that container, we don't need to re-attach event handlers manually This patch also comes with a bit of cleanup, and introduce "breaking changes" (they are breaking changes only if you happen to have custom framework plugins): 1) 'mouseover', 'mousemove', 'keypress' events are no longer listened to 'mouseover' and 'mousemove' are not used and would trigger too much events. 'keypress' is also not used, and is deprecated 2) Event handlers now takes a single parameter that is an Event object It just makes the code a lot less complicated. 3) Event handlers do not pollute the global scope anymore [1] https://learn.jquery.com/events/event-delegation/ Test plan: - Go to every page that has a MARC editor and verify that plugins still work. This includes addbiblio.pl, additem.pl, authorities.pl, neworderempty.pl, orderreceive.pl - Test plugins that use 'focus' event (for instance barcode.pl), 'blur' event (callnumber.pl) and 'click' event (almost all the others) - Test that plugins work on cloned fields/subfields --- C4/Items.pm | 2 +- Koha/FrameworkPlugin.pm | 84 ++------------- Koha/UI/Form/Builder/Item.pm | 1 + authorities/authorities.pl | 1 + cataloguing/value_builder/EXAMPLE.pl | 26 +---- .../prog/en/includes/html_helpers.inc | 6 +- .../prog/en/modules/acqui/neworderempty.tt | 2 +- .../prog/en/modules/acqui/orderreceive.tt | 2 +- .../en/modules/authorities/authorities.tt | 4 +- .../prog/en/modules/cataloguing/addbiblio.tt | 8 +- .../prog/en/modules/cataloguing/additem.tt | 2 +- koha-tmpl/intranet-tmpl/prog/js/additem.js | 14 --- koha-tmpl/intranet-tmpl/prog/js/cataloging.js | 101 ++++++++---------- 13 files changed, 72 insertions(+), 181 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index 3041d8f13e..843a98a210 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -1518,7 +1518,7 @@ sub PrepareItemrecordDisplay { my $tab= $plugin->noclick? '-1': ''; my $class= $plugin->noclick? ' disabled': ''; my $title= $plugin->noclick? 'No popup': 'Tag editor'; - $subfield_data{marc_value} = qq[...\n].$plugin->javascript; + $subfield_data{marc_value} = qq[...\n].$plugin->javascript; } else { warn $plugin->errstr; $subfield_data{marc_value} = qq(); # supply default input form diff --git a/Koha/FrameworkPlugin.pm b/Koha/FrameworkPlugin.pm index dab6c4f471..96a5945bf5 100644 --- a/Koha/FrameworkPlugin.pm +++ b/Koha/FrameworkPlugin.pm @@ -307,94 +307,24 @@ sub _process_javascript { $script =~ s/\]*\>\s*(\/\/\\s*)?\<\/script\>//s; - my $id = $params->{id}//''; - my $bind = ''; my $clickfound = 0; - my @events = qw|click focus blur change mouseover mouseout mousedown - mouseup mousemove keydown keypress keyup|; + my @events = qw|click focus blur change mousedown mouseup keydown keyup|; foreach my $ev ( @events ) { my $scan = $ev eq 'click' && $self->{oldschool}? 'clic': $ev; - if( $script =~ /function\s+($scan\w+)\s*\(([^\)]*)\)/is ) { - my ( $bl, $sl ) = $self->_add_binding( $1, $2, $ev, $id ); - $script .= $sl; - $bind .= $bl; + if( $script =~ /function\s+($scan\w+)\s*\(/is ) { + my $function_name = $1; + $script .= sprintf('registerFrameworkPluginHandler("%s", "%s", %s);', $self->name, $ev, $function_name); $clickfound = 1 if $ev eq 'click'; } } - if( !$clickfound ) { # make buttonDot do nothing - my ( $bl ) = $self->_add_binding( 'noclick', '', 'click', $id ); - $bind .= $bl; - } $self->{noclick} = !$clickfound; - $self->{javascript}= _merge_script( $id, $script, $bind ); -} - -sub _add_binding { -# adds some jQuery code for event binding: -# $bind contains lines for the actual event binding: .click, .focus, etc. -# $script contains function definitions (if needed) - my ( $self, $fname, $pars, $ev, $id ) = @_; - my ( $bind, $script ); - my $ctl= $ev eq 'click'? 'buttonDot_'.$id: $id; - #click event applies to buttonDot - - if( $pars =~ /^(e|ev|event)$/i ) { # new style event handler assumed - $bind = qq| \$("#$ctl").off('$ev').on('$ev', \{id: '$id'\}, $fname);\n|; # remove old handler if any - $script = q{}; - } elsif( $fname eq 'noclick' ) { # no click: return false, no scroll - $bind = qq| \$("#$ctl").$ev(function () { return false; });\n|; - $script = q{}; - } else { # add real event handler calling the function found - $bind = qq| \$("#$ctl").off('$ev').on('$ev', \{id: '$id'\}, ${fname}_handler);\n|; - $script = $self->_add_handler( $ev, $fname ); - } - return ( $bind, $script ); -} - -sub _add_handler { -# adds a handler with event parameter -# event.data.id is passed to the plugin function in parameters -# for the click event we always return false to prevent scrolling - my ( $self, $ev, $fname ) = @_; - my $first= $self->_first_item_par( $ev ); - my $prefix= $ev eq 'click'? '': 'return '; - my $suffix= $ev eq 'click'? "\n return false;": ''; - return <{item_style} && $self->{oldschool} && - $event=~/focus|blur|change/ ) { - return qq/'0',/; - } - return ''; -} - -sub _merge_script { -# Combine script and event bindings, enclosed in script tags. -# The BindEvents function is added to easily repeat event binding; -# this is used in additem.js for dynamically created item blocks. - my ( $id, $script, $bind ) = @_; - chomp ($script, $bind); - return <{javascript} = < +\$(document).ready(function () { $script -function BindEvents$id() { -$bind -} -\$(document).ready(function() { - BindEvents$id(); }); -HERE +JS } =head1 AUTHOR diff --git a/Koha/UI/Form/Builder/Item.pm b/Koha/UI/Form/Builder/Item.pm index 2ab9fb3bb5..942c9e8a60 100644 --- a/Koha/UI/Form/Builder/Item.pm +++ b/Koha/UI/Form/Builder/Item.pm @@ -336,6 +336,7 @@ sub generate_subfield_form { class => $class, nopopup => $plugin->noclick, javascript => $plugin->javascript, + plugin => $plugin->name, }; } else { diff --git a/authorities/authorities.pl b/authorities/authorities.pl index 761c1f529d..8f4522f3e1 100755 --- a/authorities/authorities.pl +++ b/authorities/authorities.pl @@ -200,6 +200,7 @@ sub create_input { maxlength => $max_length, javascript => $plugin->javascript, noclick => $plugin->noclick, + plugin => $plugin->name, }; } else { # warn and supply default field warn $plugin->errstr; diff --git a/cataloguing/value_builder/EXAMPLE.pl b/cataloguing/value_builder/EXAMPLE.pl index 691e69f8e2..31319494e1 100755 --- a/cataloguing/value_builder/EXAMPLE.pl +++ b/cataloguing/value_builder/EXAMPLE.pl @@ -65,37 +65,19 @@ my $builder= sub { |; }; diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc index 872388b317..b53aed5a41 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc @@ -144,11 +144,11 @@ [% IF mv.readonly %] [% ELSE %] - + [% IF ( mv.nopopup ) %] - ... + ... [% ELSE %] - ... + ... [% END %] [% UNLESS no_plugin %][%# FIXME - from batchMod-edit, jQuery is included at the end of the template and cataloguing plugins are not working in this situation %] [%- mv.javascript | $raw -%] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt index a8aa424cfe..3a4dcb40ed 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt @@ -545,7 +545,7 @@
The autoBarcode system preference is set to [% Koha.Preference('autoBarcode') | html %] and items with blank barcodes will have barcodes generated upon save to database
[% END %] -
+
[% END %][%# | html UNLESS subscriptionid %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt index bbd320328d..af979e48b2 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt @@ -243,7 +243,7 @@ used

[% END %] -
+
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/authorities.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/authorities.tt index 230b683801..386510ab6e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/authorities.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/authorities.tt @@ -473,7 +473,7 @@
[% END # /IF duplicateauthid %] -
+ [% INCLUDE 'csrf-token.inc' %] @@ -767,7 +767,7 @@ [% IF mv.noclick %] ... [% ELSE %] - ... + ... [% END %] [% mv.javascript | $raw %] [% END #/IF ( mv.type == 'text1' ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt index 9a41b4d461..24b2ff99c3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt @@ -843,7 +843,7 @@ $(document).ready(function(){
-
+
[% INCLUDE 'messages.inc' %] @@ -1190,7 +1190,7 @@ $(document).ready(function(){ [% END %] [% ELSIF ( mv.type == 'text_complex' ) %] - + [% mv.javascript | $raw %] [% ELSIF ( mv.type == 'hidden' ) %] @@ -1242,9 +1242,9 @@ $(document).ready(function(){ [% ELSE %] [% IF mv.plugin == "upload.pl" %] - Upload + Upload [% ELSE %] - Tag editor + Tag editor [% END %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt index 57e8b73173..a85a1a0d5f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt @@ -188,7 +188,7 @@
-
+
[% INCLUDE 'csrf-token.inc' %] diff --git a/koha-tmpl/intranet-tmpl/prog/js/additem.js b/koha-tmpl/intranet-tmpl/prog/js/additem.js index 099c32d6cc..bc6b1182c8 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/additem.js +++ b/koha-tmpl/intranet-tmpl/prog/js/additem.js @@ -227,24 +227,10 @@ function cloneItemBlock(index, unique_item_fields, callback) { var cloneIndex = "itemblock"+random; callback(cloneIndex); } - BindPluginEvents(data); } }); } -function BindPluginEvents(data) { -// the script tag in data for plugins contains a document ready that binds -// the events for the plugin -// when we append, this code does not get executed anymore; so we do it here - var events= data.match(/BindEventstag_\d+_subfield_._\d+/g); - if ( events == null ) return; - for(var i=0; i