Koha-community/Koha
15
7
Fork 0
Code Issues Releases Wiki Activity

Bug 30975: Use event delegation for framework plugins

Browse source 
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 <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Julian Maurice 2023-02-03 12:00:39 +01:00 committed by Katrin Fischer
parent 3bf4121b13
commit f000278b32
Signed by: kfischer
GPG key ID: 0EF6E2C03357A834
13 changed files with 78 additions and 192 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
C4/Items.pm
View file

@ -1608,7 +1608,7 @@ sub PrepareItemrecordDisplay {
my $class = $plugin->noclick ? ' disabled' : '';
my $title = $plugin->noclick ? 'No popup' : 'Tag editor';
$subfield_data{marc_value} =
qq[<input type="text" id="$subfield_data{id}" name="field_value" class="input_marceditor" size="50" maxlength="$maxlength" value="$defaultvalue" /><a href="#" id="buttonDot_$subfield_data{id}" class="buttonDot $class" title="$title">...</a>\n]
qq[<input type="text" id="$subfield_data{id}" name="field_value" class="input_marceditor framework_plugin" size="50" maxlength="$maxlength" value="$defaultvalue" data-plugin="$plugin->{name}" /><a href="#" id="buttonDot_$subfield_data{id}" class="buttonDot $class" title="$title" data-plugin="$plugin->{name}">...</a>\n]
. $plugin->javascript;
} else {
warn $plugin->errstr;

91
Koha/FrameworkPlugin.pm
View file

@ -316,101 +316,24 @@ sub _process_javascript {
$script =~ s/\<script[^>]*\>\s*(\/\/\<!\[CDATA\[)?\s*//s;
$script =~ 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 <<HERE;
function ${fname}_handler(event) {
$prefix$fname(${first}event.data.id);$suffix
}
HERE
}
sub _first_item_par {
my ( $self, $event ) = @_;
# needed for backward compatibility
# js event functions in old style item plugins have an extra parameter
# BUT.. not for all events (exceptions provide employment :)
if ( $self->{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 <<HERE;
$self->{javascript} = <<JS;
<script>
\$(document).ready(function () {
$script
function BindEvents$id() {
$bind
}
\$(document).ready(function() {
BindEvents$id();
});
</script>
HERE
JS
}
=head1 AUTHOR

1
Koha/UI/Form/Builder/Item.pm
View file

@ -329,6 +329,7 @@ sub generate_subfield_form {
class => $class,
nopopup => $plugin->noclick,
javascript => $plugin->javascript,
plugin => $plugin->name,
};
} else {
warn $plugin->errstr;

1
authorities/authorities.pl
View file

@ -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;

26
cataloguing/value_builder/EXAMPLE.pl
View file

@ -65,37 +65,19 @@ my $builder = sub {
<script>
function Focus$id(event) {
if( \$('#'+event.data.id).val()=='' ) {
\$('#'+event.data.id).val('EXAMPLE:');
\$('#'+event.data.id).val('Focus');
}
}
function MouseOver$id(event) {
return Focus$id(event);
/* just redirecting it to Focus for the same effect */
}
function KeyPress$id(event) {
if( event.which == 64 ) { /* at character */
var f= \$('#'+event.data.id).val();
\$('#'+event.data.id).val( f + 'AT' );
return false; /* prevents getting the @ character back too */
function Blur$id(event) {
if( \$('#'+event.data.id).val()=='' ) {
\$('#'+event.data.id).val('Blur');
}
}
function Change$id(event) {
var colors= [ 'rgb(0, 0, 255)', 'rgb(0, 128, 0)', 'rgb(255, 0, 0)' ];
var curcol= \$('#'+event.data.id).css('color');
var i= Math.floor( Math.random() * 3 );
if( colors[i]==curcol ) {
i= (i + 1)%3;
}
var f= \$('#'+event.data.id).css('color',colors[i]);
}
function Click$id(event) {
var fieldvalue=\$('#'+event.data.id).val();
window.open(\"../cataloguing/plugin_launcher.pl?plugin_name=EXAMPLE.pl&index=\"+event.data.id+\"&result=\"+fieldvalue,\"tag_editor\",'width=700,height=700,toolbar=false,scrollbars=yes');
return false; /* prevents scrolling */
}
</script>|;
};

14
koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc
View file

@ -173,11 +173,19 @@
readonly="readonly"
/>
[% ELSE %]
<input type="text" id="[%- mv.id | html -%]" name="[% kohafield | html %]" class="input_marceditor [% kohafield | html %]" maxlength="[%- mv.maxlength | html -%]" value="[%- mv.value | html -%]" />
<input
type="text"
id="[%- mv.id | html -%]"
name="[% kohafield | html %]"
class="input_marceditor framework_plugin [% kohafield | html %]"
maxlength="[%- mv.maxlength | html -%]"
value="[%- mv.value | html -%]"
data-plugin="[% mv.plugin | html %]"
/>
[% IF ( mv.nopopup ) %]
<a href="#" id="buttonDot_[%- mv.id | html -%]" class="[%- mv.class | html -%]" title="No popup">...</a>
<a href="#" id="buttonDot_[%- mv.id | html -%]" class="[%- mv.class | html -%]" title="No popup" data-plugin="[% mv.plugin | html %]">...</a>
[% ELSE %]
<a href="#" id="buttonDot_[%- mv.id | html -%]" class="[%- mv.class | html -%]" title="Tag editor">...</a>
<a href="#" id="buttonDot_[%- mv.id | html -%]" class="[%- mv.class | html -%]" title="Tag editor" data-plugin="[% mv.plugin | html %]">...</a>
[% 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 %]

2
koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
View file

@ -539,7 +539,7 @@
<div class="alert alert-info">The autoBarcode system preference is set to [% Koha.Preference('autoBarcode') | html %] and items with blank barcodes will have barcodes generated upon save to database</div>
[% END %]
<div id="outeritemblock"></div>
<div id="outeritemblock" class="marc_editor"></div>
</fieldset>
[% END %][%# | html UNLESS subscriptionid %]
[% END %][%# IF (AcqCreateItemOrdering) %]

2
koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt
View file

@ -236,7 +236,7 @@
[% IF ( NoACQframework ) %]
<p class="required"> No ACQ framework, using default. You should create a framework with code ACQ, the items framework would be used </p>
[% END %]
<div id="outeritemblock"></div>
<div id="outeritemblock" class="marc_editor"></div>
</div>
</div>
<div id="acq-create-ordering">

4
koha-tmpl/intranet-tmpl/prog/en/modules/authorities/authorities.tt
View file

@ -449,7 +449,7 @@
</div>
[% END # /IF duplicateauthid %]
<form method="post" id="f" name="f" action="/cgi-bin/koha/authorities/authorities.pl">
<form method="post" id="f" name="f" action="/cgi-bin/koha/authorities/authorities.pl" class="marc_editor">
[% INCLUDE 'csrf-token.inc' %]
<input type="hidden" name="op" value="cud-add" />
<input type="hidden" name="original_op" value="[% op | html %]" />
@ -755,7 +755,7 @@
[% IF mv.noclick %]
<a href="#" class="buttonDot tag_editor disabled" tabindex="-1" title="No popup">...</a>
[% ELSE %]
<a href="#" id="buttonDot_[% mv.id | html %]" class="buttonDot tag_editor" title="Tag editor">...</a>
<a href="#" id="buttonDot_[% mv.id | html %]" class="buttonDot tag_editor" title="Tag editor" data-plugin="[% mv.plugin | html %]">...</a>
[% END %]
[% mv.javascript | $raw %]
[% END #/IF ( mv.type == 'text1' ) %]

8
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
View file

@ -889,7 +889,7 @@
window.close();
</script>
[% ELSE %]
<form method="post" name="f" id="f" action="/cgi-bin/koha/cataloguing/addbiblio.pl" onsubmit="return Check();">
<form method="post" name="f" id="f" action="/cgi-bin/koha/cataloguing/addbiblio.pl" onsubmit="return Check();" class="marc_editor">
[% INCLUDE 'csrf-token.inc' %]
<input type="hidden" value="[% IF ( biblionumber ) %]view[% ELSE %]items[% END %]" id="redirect" name="redirect" />
<input type="hidden" value="" id="current_tab" name="current_tab" />
@ -1165,7 +1165,7 @@
[% END %]
[% ELSIF ( mv.type == 'text_complex' ) %]
<input type="text" id="[%- mv.id | html -%]" name="[%- mv.name | html -%]" value="[%- mv.value | html -%]" class="input_marceditor framework_plugin" tabindex="1" size="[%- mv.size | html -%]" maxlength="[%- mv.maxlength | html -%]" />
<input type="text" id="[%- mv.id | html -%]" name="[%- mv.name | html -%]" value="[%- mv.value | html -%]" class="input_marceditor framework_plugin" tabindex="1" size="[%- mv.size | html -%]" maxlength="[%- mv.maxlength | html -%]" data-plugin="[% mv.plugin | html %]" />
[% mv.javascript | $raw %]
[% ELSIF ( mv.type == 'hidden' ) %]
<input tabindex="1" type="hidden" id="[%- mv.id | html -%]" name="[%- mv.name | html -%]" size="[%- mv.size | html -%]" maxlength="[%- mv.maxlength | html -%]" value="[%- mv.value | html -%]" />
@ -1217,9 +1217,9 @@
<span class="buttonDot tag_editor disabled" tabindex="-1" title="Field autofilled by plugin"></span>
[% ELSE %]
[% IF mv.plugin == "upload.pl" %]
<a href="#" id="buttonDot_[% mv.id | html %]" class="tag_editor upload framework_plugin" tabindex="1"><i class="fa fa-upload" aria-hidden="true"></i> Upload</a>
<a href="#" id="buttonDot_[% mv.id | html %]" class="tag_editor upload framework_plugin" tabindex="1" data-plugin="[% mv.plugin | html %]"><i class="fa fa-upload" aria-hidden="true"></i> Upload</a>
[% ELSE %]
<a href="#" id="buttonDot_[% mv.id | html %]" class="buttonDot tag_editor framework_plugin" tabindex="1" title="Tag editor">Tag editor</a>
<a href="#" id="buttonDot_[% mv.id | html %]" class="buttonDot tag_editor framework_plugin" tabindex="1" title="Tag editor" data-plugin="[% mv.plugin | html %]">Tag editor</a>
[% END %]
[% END %]
</span>

2
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt
View file

@ -225,7 +225,7 @@
<div class="row">
<div class="col-md-2 order-sm-2 order-md-1"> [% INCLUDE 'biblio-view-menu.inc' %] </div>
<div class="col-md-10 order-md-2 order-sm-1">
<div id="cataloguing_additem_newitem" class="item_edit_form page-section">
<div id="cataloguing_additem_newitem" class="item_edit_form page-section marc_editor">
<form id="f" method="post" action="/cgi-bin/koha/cataloguing/additem.pl?biblionumber=[% biblio.biblionumber | html %]" name="f">
[% INCLUDE 'csrf-token.inc' %]
<input type="hidden" name="op" value="[% op | html %]" />

16
koha-tmpl/intranet-tmpl/prog/js/additem.js
View file

@ -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);

101
koha-tmpl/intranet-tmpl/prog/js/cataloging.js
View file

@ -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);
});