From 6d80770ba47bc232c422a82915863706252f052e Mon Sep 17 00:00:00 2001 From: Cesar G Date: Sun, 14 Sep 2014 14:54:08 -0700 Subject: [PATCH 1/4] [ticket/13018] Remove duplicate logic in confirm box and alert box handling. This ensures a more consistent handling of the various closing/opening actions and fixes some bugs where certain event handlers are not turned off in certain instances. PHPBB3-13018 --- phpBB/assets/javascript/core.js | 168 +++++++++++++++----------------- 1 file changed, 78 insertions(+), 90 deletions(-) diff --git a/phpBB/assets/javascript/core.js b/phpBB/assets/javascript/core.js index 802b4dd7c8..77f81358c8 100644 --- a/phpBB/assets/javascript/core.js +++ b/phpBB/assets/javascript/core.js @@ -79,41 +79,39 @@ phpbb.alert = function(title, msg, fadedark) { $alert.find('.alert_title').html(title); $alert.find('.alert_text').html(msg); - if (!$dark.is(':visible')) { - $dark.fadeIn(phpbb.alertTime); - } - - $alert.on('click', function(e) { - e.stopPropagation(); - }); - $dark.one('click', function(e) { - var fade; - - $alert.find('.alert_close').off('click'); - fade = (typeof fadedark !== 'undefined' && !fadedark) ? $alert : $dark; - fade.fadeOut(phpbb.alertTime, function() { - $alert.hide(); - }); - - e.preventDefault(); - e.stopPropagation(); - }); - - $(document).keydown(function(e) { - if ((e.keyCode === keymap.ENTER || e.keyCode === keymap.ESC) && $dark.is(':visible')) { - $dark.trigger('click'); - - e.preventDefault(); - e.stopPropagation(); + $(document).on('keydown.phpbb.alert', function(e) { + if (e.keyCode === keymap.ENTER || e.keyCode === keymap.ESC) { + closeBox(true, e, true); } }); - $alert.find('.alert_close').one('click', function(e) { - $dark.trigger('click'); - - e.preventDefault(); + $dark.one('click', function(e) { + closeBox(true, e, true); }); + $alert.find('.alert_close').one('click', function(e) { + closeBox(true, e, false); + }); + + var closeBox = function(fadedark, event, stopPropagation) { + phpbb.alert.close($alert, fadedark, event, stopPropagation); + }; + + phpbb.alert.open($alert); + + return $alert; +}; + +/** +* Handler for opening an alert box. +* +* @param jQuery $alert jQuery object. +*/ +phpbb.alert.open = function($alert) { + if (!$dark.is(':visible')) { + $dark.fadeIn(phpbb.alertTime); + } + if ($loadingIndicator.is(':visible')) { $loadingIndicator.fadeOut(phpbb.alertTime, function() { $dark.append($alert); @@ -128,7 +126,35 @@ phpbb.alert = function(title, msg, fadedark) { $dark.fadeIn(phpbb.alertTime); } - return $alert; + $alert.on('click', function(e) { + e.stopPropagation(); + }); +}; + +/** +* Handler for closing an alert box. +* +* @param jQuery $alert jQuery object. +* @param bool fadedark Whether to remove dark background. +* @param object event Event object. +* @param bool stopPropagation Whether to stop event's propagation. +*/ +phpbb.alert.close = function($alert, fadedark, event, stopPropagation) { + var $fade = (fadedark) ? $dark : $alert; + + $fade.fadeOut(phpbb.alertTime, function() { + $alert.hide(); + }); + + $alert.find('.alert_close').off('click'); + $(document).off('keydown.phpbb.alert'); + + if (event) { + event.preventDefault(); + if (stopPropagation) { + event.stopPropagation(); + } + } }; /** @@ -147,77 +173,39 @@ phpbb.alert = function(title, msg, fadedark) { phpbb.confirm = function(msg, callback, fadedark) { var $confirmDiv = $('#phpbb_confirm'); $confirmDiv.find('.alert_text').html(msg); + fadedark = (typeof fadedark === 'undefined') ? true : fadedark; - if (!$dark.is(':visible')) { - $dark.fadeIn(phpbb.alertTime); - } + $(document).on('keydown.phpbb.alert', function(e) { + if (e.keyCode === keymap.ENTER || e.keyCode === keymap.ESC) { + var name = (e.keyCode === keymap.ENTER) ? 'confirm' : 'cancel'; - $confirmDiv.on('click', function(e) { - e.stopPropagation(); - }); - - var clickHandler = function(e) { - var res = this.name === 'confirm'; - var $fade = (typeof fadedark !== 'undefined' && !fadedark && res) ? $confirmDiv : $dark; - $fade.fadeOut(phpbb.alertTime, function() { - $confirmDiv.hide(); - }); - $confirmDiv.find('input[type="button"]').off('click', clickHandler); - callback(res); - - if (e) { + $('input[name="' + name + '"]').trigger('click'); e.preventDefault(); e.stopPropagation(); } - }; - $confirmDiv.find('input[type="button"]').one('click', clickHandler); + }); + + $confirmDiv.find('input[type="button"]').one('click.phpbb.confirmbox', function(e) { + var confirmed = this.name === 'confirm', + fadedark = fadedark || !confirmed; + closeBox(fadedark, confirmed, e, true); + }); $dark.one('click', function(e) { - $confirmDiv.find('.alert_close').off('click'); - $dark.fadeOut(phpbb.alertTime, function() { - $confirmDiv.hide(); - }); - callback(false); - - e.preventDefault(); - e.stopPropagation(); - }); - - $(document).on('keydown', function(e) { - if (e.keyCode === keymap.ENTER) { - $('input[name="confirm"]').trigger('click'); - e.preventDefault(); - e.stopPropagation(); - } else if (e.keyCode === keymap.ESC) { - $('input[name="cancel"]').trigger('click'); - e.preventDefault(); - e.stopPropagation(); - } + closeBox(true, false, e, true); }); $confirmDiv.find('.alert_close').one('click', function(e) { - var $fade = (typeof fadedark !== 'undefined' && fadedark) ? $confirmDiv : $dark; - $fade.fadeOut(phpbb.alertTime, function() { - $confirmDiv.hide(); - }); - callback(false); - - e.preventDefault(); + closeBox(true, false, e, false); }); - if ($loadingIndicator.is(':visible')) { - $loadingIndicator.fadeOut(phpbb.alertTime, function() { - $dark.append($confirmDiv); - $confirmDiv.fadeIn(phpbb.alertTime); - }); - } else if ($dark.is(':visible')) { - $dark.append($confirmDiv); - $confirmDiv.fadeIn(phpbb.alertTime); - } else { - $dark.append($confirmDiv); - $confirmDiv.show(); - $dark.fadeIn(phpbb.alertTime); - } + var closeBox = function(fadedark, confirmed, event, stopPropagation) { + $confirmDiv.find('input[type="button"]').off('click.phpbb.confirmbox'); + callback(confirmed); + phpbb.alert.close($confirmDiv, fadedark, event, stopPropagation); + }; + + phpbb.alert.open($confirmDiv); return $confirmDiv; }; From 5034b3ad7db2c56d88da2cb62b141ff514db7b1f Mon Sep 17 00:00:00 2001 From: Cesar G Date: Sun, 14 Sep 2014 15:49:18 -0700 Subject: [PATCH 2/4] [ticket/13018] Reduce the delta further. The callback does not actually do anything when cancelling the confirmation box so we can avoid calling it altogether when cancel is clicked. PHPBB3-13018 --- phpBB/assets/javascript/core.js | 42 +++++++++++---------------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/phpBB/assets/javascript/core.js b/phpBB/assets/javascript/core.js index 77f81358c8..de21cc9608 100644 --- a/phpBB/assets/javascript/core.js +++ b/phpBB/assets/javascript/core.js @@ -81,22 +81,9 @@ phpbb.alert = function(title, msg, fadedark) { $(document).on('keydown.phpbb.alert', function(e) { if (e.keyCode === keymap.ENTER || e.keyCode === keymap.ESC) { - closeBox(true, e, true); + phpbb.alert.close($alert, true, e, true); } }); - - $dark.one('click', function(e) { - closeBox(true, e, true); - }); - - $alert.find('.alert_close').one('click', function(e) { - closeBox(true, e, false); - }); - - var closeBox = function(fadedark, event, stopPropagation) { - phpbb.alert.close($alert, fadedark, event, stopPropagation); - }; - phpbb.alert.open($alert); return $alert; @@ -129,6 +116,14 @@ phpbb.alert.open = function($alert) { $alert.on('click', function(e) { e.stopPropagation(); }); + + $dark.one('click', function(e) { + phpbb.alert.close($alert, true, e, true); + }); + + $alert.find('.alert_close').one('click', function(e) { + phpbb.alert.close($alert, true, e, false); + }); }; /** @@ -188,22 +183,13 @@ phpbb.confirm = function(msg, callback, fadedark) { $confirmDiv.find('input[type="button"]').one('click.phpbb.confirmbox', function(e) { var confirmed = this.name === 'confirm', fadedark = fadedark || !confirmed; - closeBox(fadedark, confirmed, e, true); - }); - $dark.one('click', function(e) { - closeBox(true, false, e, true); - }); - - $confirmDiv.find('.alert_close').one('click', function(e) { - closeBox(true, false, e, false); - }); - - var closeBox = function(fadedark, confirmed, event, stopPropagation) { + if (confirmed) { + callback(true); + } $confirmDiv.find('input[type="button"]').off('click.phpbb.confirmbox'); - callback(confirmed); - phpbb.alert.close($confirmDiv, fadedark, event, stopPropagation); - }; + phpbb.alert.close($confirmDiv, fadedark, e, true); + }); phpbb.alert.open($confirmDiv); From 2b4807b1162a3956e3d3f9752056657fb1678d24 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Mon, 15 Sep 2014 07:43:18 -0700 Subject: [PATCH 3/4] [ticket/13018] Do not handle events in close function. PHPBB3-13018 --- phpBB/assets/javascript/core.js | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/phpBB/assets/javascript/core.js b/phpBB/assets/javascript/core.js index de21cc9608..a06f00d8ae 100644 --- a/phpBB/assets/javascript/core.js +++ b/phpBB/assets/javascript/core.js @@ -81,7 +81,9 @@ phpbb.alert = function(title, msg, fadedark) { $(document).on('keydown.phpbb.alert', function(e) { if (e.keyCode === keymap.ENTER || e.keyCode === keymap.ESC) { - phpbb.alert.close($alert, true, e, true); + phpbb.alert.close($alert, true); + e.preventDefault(); + e.stopPropagation(); } }); phpbb.alert.open($alert); @@ -118,11 +120,14 @@ phpbb.alert.open = function($alert) { }); $dark.one('click', function(e) { - phpbb.alert.close($alert, true, e, true); + phpbb.alert.close($alert, true); + e.preventDefault(); + e.stopPropagation(); }); $alert.find('.alert_close').one('click', function(e) { - phpbb.alert.close($alert, true, e, false); + phpbb.alert.close($alert, true); + e.preventDefault(); }); }; @@ -131,10 +136,8 @@ phpbb.alert.open = function($alert) { * * @param jQuery $alert jQuery object. * @param bool fadedark Whether to remove dark background. -* @param object event Event object. -* @param bool stopPropagation Whether to stop event's propagation. */ -phpbb.alert.close = function($alert, fadedark, event, stopPropagation) { +phpbb.alert.close = function($alert, fadedark) { var $fade = (fadedark) ? $dark : $alert; $fade.fadeOut(phpbb.alertTime, function() { @@ -143,13 +146,6 @@ phpbb.alert.close = function($alert, fadedark, event, stopPropagation) { $alert.find('.alert_close').off('click'); $(document).off('keydown.phpbb.alert'); - - if (event) { - event.preventDefault(); - if (stopPropagation) { - event.stopPropagation(); - } - } }; /** @@ -188,7 +184,10 @@ phpbb.confirm = function(msg, callback, fadedark) { callback(true); } $confirmDiv.find('input[type="button"]').off('click.phpbb.confirmbox'); - phpbb.alert.close($confirmDiv, fadedark, e, true); + phpbb.alert.close($confirmDiv, fadedark); + + e.preventDefault(); + e.stopPropagation(); }); phpbb.alert.open($confirmDiv); From 4ae447b20500f3122e7ff57c365bc699ef932e85 Mon Sep 17 00:00:00 2001 From: Cesar G Date: Mon, 15 Sep 2014 07:44:02 -0700 Subject: [PATCH 4/4] [ticket/13018] Clean up and fix some logic. PHPBB3-13018 --- phpBB/assets/javascript/core.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/phpBB/assets/javascript/core.js b/phpBB/assets/javascript/core.js index a06f00d8ae..5fe78cf052 100644 --- a/phpBB/assets/javascript/core.js +++ b/phpBB/assets/javascript/core.js @@ -164,7 +164,7 @@ phpbb.alert.close = function($alert, fadedark) { phpbb.confirm = function(msg, callback, fadedark) { var $confirmDiv = $('#phpbb_confirm'); $confirmDiv.find('.alert_text').html(msg); - fadedark = (typeof fadedark === 'undefined') ? true : fadedark; + fadedark = fadedark || true; $(document).on('keydown.phpbb.alert', function(e) { if (e.keyCode === keymap.ENTER || e.keyCode === keymap.ESC) { @@ -177,14 +177,13 @@ phpbb.confirm = function(msg, callback, fadedark) { }); $confirmDiv.find('input[type="button"]').one('click.phpbb.confirmbox', function(e) { - var confirmed = this.name === 'confirm', - fadedark = fadedark || !confirmed; + var confirmed = this.name === 'confirm'; if (confirmed) { callback(true); } $confirmDiv.find('input[type="button"]').off('click.phpbb.confirmbox'); - phpbb.alert.close($confirmDiv, fadedark); + phpbb.alert.close($confirmDiv, fadedark || !confirmed); e.preventDefault(); e.stopPropagation();