From c4f7aaa465accdc1f49259f8be11db62e073dbc5 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Fri, 3 Feb 2023 12:00:39 +0100 Subject: [PATCH] Bug 30975: Use event delegation for framework plugins This is to avoid using private jQuery method _data. 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. 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 Rebased-by: Victor Grousset/tuxayo Signed-off-by: Marcel de Rooy Signed-off-by: David Cook --- C4/Items.pm | 2 +- Koha/FrameworkPlugin.pm | 91 ++-------------- 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 | 16 --- koha-tmpl/intranet-tmpl/prog/js/cataloging.js | 101 ++++++++---------- 13 files changed, 70 insertions(+), 192 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index c0375939d6..f1ac3616a0 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -1608,7 +1608,7 @@ sub PrepareItemrecordDisplay { my $class = $plugin->noclick ? ' disabled' : ''; my $title = $plugin->noclick ? 'No popup' : 'Tag editor'; $subfield_data{marc_value} = - qq[...\n] + qq[...\n] . $plugin->javascript; } else { warn $plugin->errstr; diff --git a/Koha/FrameworkPlugin.pm b/Koha/FrameworkPlugin.pm index 842e660233..57555355d0 100644 --- a/Koha/FrameworkPlugin.pm +++ b/Koha/FrameworkPlugin.pm @@ -316,101 +316,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 9ec651e186..7a2f54466f 100644 --- a/Koha/UI/Form/Builder/Item.pm +++ b/Koha/UI/Form/Builder/Item.pm @@ -329,6 +329,7 @@ sub generate_subfield_form { class => $class, nopopup => $plugin->noclick, javascript => $plugin->javascript, + plugin => $plugin->name, }; } else { warn $plugin->errstr; diff --git a/authorities/authorities.pl b/authorities/authorities.pl index 74c22d54f6..3385a260fa 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 713bfa41df..6a952845b5 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 aa92f7bb61..4989a4a6e3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc @@ -173,11 +173,11 @@ readonly="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 %] 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 f08f0d77ed..20639644a8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt @@ -539,7 +539,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 %] [% END %][%# IF (AcqCreateItemOrdering) %] 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 3dab4dc0d3..e18662c856 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt @@ -236,7 +236,7 @@ [% IF ( NoACQframework ) %]

No ACQ framework, using default. You should create a framework with code ACQ, the items framework would be 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 00a28b4674..75c116c950 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/authorities.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/authorities.tt @@ -449,7 +449,7 @@
[% END # /IF duplicateauthid %] -
+ [% INCLUDE 'csrf-token.inc' %] @@ -755,7 +755,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 0748c11ee7..9fff1c915e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt @@ -889,7 +889,7 @@ window.close(); [% ELSE %] - + [% INCLUDE 'csrf-token.inc' %] @@ -1165,7 +1165,7 @@ [% END %] [% ELSIF ( mv.type == 'text_complex' ) %] - + [% mv.javascript | $raw %] [% ELSIF ( mv.type == 'hidden' ) %] @@ -1217,9 +1217,9 @@ [% 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 ca8e2467a6..90fe28057e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt @@ -225,7 +225,7 @@
[% INCLUDE 'biblio-view-menu.inc' %]
-
+
[% 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 462e8db8f3..5ce8be0349 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/additem.js +++ b/koha-tmpl/intranet-tmpl/prog/js/additem.js @@ -337,26 +337,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 < events.length; i++) { - window[events[i]](); - if (i < events.length - 1 && events[i] == events[i + 1]) { - i++; - } - // normally we find the function name twice - } -} - function clearItemBlock(node) { var index = $(node).closest("div").attr("id"); var block = $("#" + index); diff --git a/koha-tmpl/intranet-tmpl/prog/js/cataloging.js b/koha-tmpl/intranet-tmpl/prog/js/cataloging.js index 2fee76f4d0..95e2a06332 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/cataloging.js +++ b/koha-tmpl/intranet-tmpl/prog/js/cataloging.js @@ -1,5 +1,5 @@ /* global __ */ -/* exported openAuth ExpandField CloneField CloneSubfield UnCloneField CloneItemSubfield CheckMandatorySubfields */ +/* exported openAuth ExpandField CloneField CloneSubfield UnCloneField CloneItemSubfield CheckMandatorySubfields registerFrameworkPluginHandler */ /* * Unified file for catalogue edition @@ -253,8 +253,6 @@ function CloneField(index, hideMarc, advancedMARCEditor) { var inputs = divs[i].getElementsByTagName("input"); var id_input = ""; - var olddiv; - var oldcontrol; for (j = 0; j < inputs.length; j++) { if ( @@ -323,11 +321,6 @@ function CloneField(index, hideMarc, advancedMARCEditor) { } } } - if ($(inputs[1]).hasClass("framework_plugin")) { - olddiv = original.getElementsByTagName("li")[i]; - oldcontrol = olddiv.getElementsByTagName("input")[1]; - AddEventHandlers(oldcontrol, inputs[1], id_input); - } } // when cloning a subfield, re set its label too. try { @@ -380,24 +373,10 @@ function CloneField(index, hideMarc, advancedMARCEditor) { if (buttonDot) { // 2 possibilities : try { - if ($(buttonDot).hasClass("framework_plugin")) { - olddiv = original.getElementsByTagName("li")[i]; - oldcontrol = - olddiv.getElementsByTagName("a")[0]; - AddEventHandlers( - oldcontrol, - buttonDot, - id_input - ); - } - try { - // do not copy the script section. - var script = - spans[0].getElementsByTagName("script")[0]; - spans[0].removeChild(script); - } catch (e) { - // do nothing if there is no script - } + // do not copy the script section. + var script = + spans[0].getElementsByTagName("script")[0]; + spans[0].removeChild(script); } catch (e) { // } @@ -493,7 +472,6 @@ function CloneSubfield(index, advancedMARCEditor) { var selects = clone.getElementsByTagName("select"); var textareas = clone.getElementsByTagName("textarea"); var linkid; - var oldcontrol; // input var id_input = ""; @@ -513,12 +491,6 @@ function CloneSubfield(index, advancedMARCEditor) { linkid = id_input; } - // Plugin input - if ($(inputs[1]).hasClass("framework_plugin")) { - oldcontrol = original.getElementsByTagName("input")[1]; - AddEventHandlers(oldcontrol, inputs[1], linkid); - } - // select for (i = 0, len = selects.length; i < len; i++) { id_input = selects[i].getAttribute("id") + new_key; @@ -551,13 +523,6 @@ function CloneSubfield(index, advancedMARCEditor) { linkid = id_input; } - // Handle click event on buttonDot for plugin - var links = clone.getElementsByTagName("a"); - if ($(links[0]).hasClass("framework_plugin")) { - oldcontrol = original.getElementsByTagName("a")[0]; - AddEventHandlers(oldcontrol, links[0], linkid); - } - if (advancedMARCEditor == "0") { // when cloning a subfield, reset its label too. var label = clone.getElementsByTagName("label")[0]; @@ -605,23 +570,6 @@ function CloneSubfield(index, advancedMARCEditor) { clone.querySelectorAll("input.input_marceditor").value = ""; } -function AddEventHandlers(oldcontrol, newcontrol, newinputid) { - // This function is a helper for CloneField and CloneSubfield. - // It adds the event handlers from oldcontrol to newcontrol. - // newinputid is the id attribute of the cloned controlling input field - // Note: This code depends on the jQuery data for events; this structure - // is moved to _data as of jQuery 1.8. - var ev = $._data(oldcontrol, "events"); - if (typeof ev != "undefined") { - $.each(ev, function (prop, val) { - $.each(val, function (prop2, val2) { - $(newcontrol).off(val2.type); - $(newcontrol).on(val2.type, { id: newinputid }, val2.handler); - }); - }); - } -} - /** * This function removes or clears unwanted subfields */ @@ -850,3 +798,42 @@ $(document).ready(function () { }, }); }); + +Koha.frameworkPlugins ||= {}; +function registerFrameworkPluginHandler(name, eventType, handler) { + // 'focus' and 'blur' events do not bubble, + // so we have to use 'focusin' and 'focusout' instead + if (eventType === 'focus') eventType = 'focusin'; + else if (eventType === 'blur') eventType = 'focusout'; + + Koha.frameworkPlugins[name] ||= {}; + Koha.frameworkPlugins[name][eventType] ||= handler; +} +$(document).ready(function() { + function callClickPluginEventHandler (event) { + event.preventDefault(); + callPluginEventHandler.call(this, event); + } + + function callPluginEventHandler (event) { + event.stopPropagation(); + + const plugin = event.target.getAttribute('data-plugin'); + if (plugin && plugin in Koha.frameworkPlugins && event.type in Koha.frameworkPlugins[plugin]) { + event.data = {}; + if (event.target.classList.contains('buttonDot')) { + event.data.id = event.target.closest('.subfield_line').querySelector('input.input_marceditor').id; + } else { + event.data.id = event.target.id; + } + + Koha.frameworkPlugins[plugin][event.type].call(this, event); + } + } + + // We use delegated event handlers here so that dynamically added elements + // (like when cloning a field or a subfield) respond to these events + // without having to re-attach events manually + $('.marc_editor').on('click', '.buttonDot', callClickPluginEventHandler); + $('.marc_editor').on('focusin focusout change mousedown mouseup keydown keyup', 'input.input_marceditor.framework_plugin', callPluginEventHandler); +}); -- 2.39.5