Go to main content

Textpattern CMS support forum

You are not logged in. Register | Login | Help

#73 2012-06-29 19:54:32

Gocom
Developer Emeritus
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: New sections tab, multi-edit control block

Stef, please see following patch. It adds the changes I mentioned:

  • Removes verify() calls from the HTML markup, and adds it to the jQuery plugin.
  • Introduces confirmation option, by default populated with are_you_sure string. Plugins/core can disable the confirmation dialog by setting the plugin’s confirmation option to false.
  • When user cancels an option, the selected multi edit method and the step are reset/unset. This is to prevent visible process locking which happens when confirm() dialog is fired.
  • Since the method is reset when needed, button will only be shown when actually needed, in other words when there is a extra step.
Index: include/txp_plugin.php
===================================================================
--- include/txp_plugin.php	(revision 3871)
+++ include/txp_plugin.php	(working copy)
@@ -68,7 +68,7 @@
 		if ($rs and numRows($rs) > 0)
 		{
 			echo n.'<div id="'.$event.'_container" class="txp-container">';
-			echo '<form action="index.php" id="plugin_form" class="multi_edit_form" method="post" name="longform" onsubmit="return verify(\''.gTxt('are_you_sure').'\')">'.
+			echo '<form action="index.php" id="plugin_form" class="multi_edit_form" method="post" name="longform">'.

 			n.'<div class="txp-listtables">'.
 			n. startTable('', '', 'txp-list').
Index: include/txp_form.php
===================================================================
--- include/txp_form.php	(revision 3871)
+++ include/txp_form.php	(working copy)
@@ -94,12 +94,14 @@
 			$out[] = '</ul></div></div>';
 			$out[] = multi_edit($methods, 'form', 'form_multi_edit');

-			return form( join('',$out),'',"verify('".gTxt('are_you_sure')."')", 'post', '', '', 'allforms_form' ).
+			return form( join('',$out),'','', 'post', '', '', 'allforms_form' ).
 				script_js( <<<EOS
-				$('#allforms_form').txpMultiEditForm({
-					'checkbox' : 'input[name="selected_forms[]"][type=checkbox]',
-					'row' : '.plain-list li, .form-list-name',
-					'highlighted' : '.plain-list li'
+				$(document).ready(function() {
+					$('#allforms_form').txpMultiEditForm({
+						'checkbox' : 'input[name="selected_forms[]"][type=checkbox]',
+						'row' : '.plain-list li, .form-list-name',
+						'highlighted' : '.plain-list li'
+					});
 				});
 EOS
 				);
Index: include/txp_section.php
===================================================================
--- include/txp_section.php	(revision 3871)
+++ include/txp_section.php	(working copy)
@@ -163,7 +163,7 @@
 		if ($rs)
 		{
 			echo n.'<div id="'.$event.'_container" class="txp-container">';
-			echo n.n.'<form action="index.php" id="section_form" class="multi_edit_form" method="post" name="longform" onsubmit="return verify(\''.gTxt('are_you_sure').'\')">'.
+			echo n.n.'<form action="index.php" id="section_form" class="multi_edit_form" method="post" name="longform">'.

 				n.'<div class="txp-listtables">'.n.
 				n.startTable('', '', 'txp-list').
Index: include/txp_list.php
===================================================================
--- include/txp_list.php	(revision 3871)
+++ include/txp_list.php	(working copy)
@@ -189,7 +189,7 @@
 			}

 			echo n.'<div id="'.$event.'_container" class="txp-container">';
-			echo n.n.'<form name="longform" id="articles_form" class="multi_edit_form" method="post" action="index.php" onsubmit="return verify(\''.gTxt('are_you_sure').'\')">'.
+			echo n.n.'<form name="longform" id="articles_form" class="multi_edit_form" method="post" action="index.php">'.

 				n.'<div class="txp-listtables">'.
 				n.startTable('', '', 'txp-list').
Index: include/txp_discuss.php
===================================================================
--- include/txp_discuss.php	(revision 3871)
+++ include/txp_discuss.php	(working copy)
@@ -241,7 +241,7 @@
 		if ($rs)
 		{
 			echo n.'<div id="'.$event.'_container" class="txp-container">';
-			echo n.n.'<form name="longform" id="discuss_form" class="multi_edit_form" method="post" action="index.php" onsubmit="return verify(\''.gTxt('are_you_sure').'\')">'.
+			echo n.n.'<form name="longform" id="discuss_form" class="multi_edit_form" method="post" action="index.php">'.

 				n.'<div class="txp-listtables">'.
 				n.startTable('', '', 'txp-list').
Index: include/txp_category.php
===================================================================
--- include/txp_category.php	(revision 3871)
+++ include/txp_category.php	(working copy)
@@ -57,9 +57,11 @@
 		endTable(),
 		'</div>',
 		script_js( <<<EOS
-			$('.category-tree').txpMultiEditForm({
-				'row' : 'p',
-				'highlighted' : 'p'
+			$(document).ready(function() {
+				$('.category-tree').txpMultiEditForm({
+					'row' : 'p',
+					'highlighted' : 'p'
+				});
 			});
 EOS
 		));
@@ -176,7 +178,7 @@
 					join('',$array).
 					hInput('type',$area).
 					n.multi_edit($methods, 'category', 'cat_category_multiedit')
-					,'',"verify('".gTxt('are_you_sure')."')", 'post', 'category-tree', '', 'category_'.$area.'_form'
+					,'', '', 'post', 'category-tree', '', 'category_'.$area.'_form'
 				);
 		}
 		return;
Index: include/txp_file.php
===================================================================
--- include/txp_file.php	(revision 3871)
+++ include/txp_file.php	(working copy)
@@ -195,7 +195,7 @@
 			$show_authors = !has_single_author('txp_file');

 			echo n.'<div id="'.$event.'_container" class="txp-container">';
-			echo '<form name="longform" id="files_form" class="multi_edit_form" method="post" action="index.php" onsubmit="return verify(\''.gTxt('are_you_sure').'\')">'.
+			echo '<form name="longform" id="files_form" class="multi_edit_form" method="post" action="index.php">'.

 				n.'<div class="txp-listtables">'.
 				n.startTable('', '', 'txp-list').
Index: include/txp_log.php
===================================================================
--- include/txp_log.php	(revision 3871)
+++ include/txp_log.php	(working copy)
@@ -157,7 +157,7 @@
 		if ($rs)
 		{
 			echo n.'<div id="'.$event.'_container" class="txp-container">';
-			echo n.n.'<form action="index.php" id="log_form" class="multi_edit_form" method="post" name="longform" onsubmit="return verify(\''.gTxt('are_you_sure').'\')">'.
+			echo n.n.'<form action="index.php" id="log_form" class="multi_edit_form" method="post" name="longform">'.

 				n.'<div class="txp-listtables">'.
 				n.startTable('', '', 'txp-list').
Index: include/txp_image.php
===================================================================
--- include/txp_image.php	(revision 3871)
+++ include/txp_image.php	(working copy)
@@ -178,7 +178,7 @@
 			$show_authors = !has_single_author('txp_image');

 			echo n.'<div id="'.$event.'_container" class="txp-container">';
-			echo n.n.'<form name="longform" id="images_form" class="multi_edit_form" method="post" action="index.php" onsubmit="return verify(\''.gTxt('are_you_sure').'\')">'.
+			echo n.n.'<form name="longform" id="images_form" class="multi_edit_form" method="post" action="index.php">'.

 				n.'<div class="txp-listtables">'.
 				n.startTable('', '', 'txp-list').
Index: include/txp_admin.php
===================================================================
--- include/txp_admin.php	(revision 3871)
+++ include/txp_admin.php	(working copy)
@@ -357,7 +357,7 @@
 			if ($rs)
 			{
 				echo n.'<div id="users_container" class="txp-container">';
-				echo '<form action="index.php" id="users_form" class="multi_edit_form" method="post" name="longform" onsubmit="return verify(\''.gTxt('are_you_sure').'\')">'.
+				echo '<form action="index.php" id="users_form" class="multi_edit_form" method="post" name="longform">'.

 				n.'<div class="txp-listtables">'.
 				n.startTable('', '', 'txp-list').
Index: include/txp_link.php
===================================================================
--- include/txp_link.php	(revision 3871)
+++ include/txp_link.php	(working copy)
@@ -165,7 +165,7 @@
 			$show_authors = !has_single_author('txp_link');

 			echo n.'<div id="'.$event.'_container" class="txp-container">';
-			echo n.n.'<form action="index.php" id="links_form" class="multi_edit_form" method="post" name="longform" onsubmit="return verify(\''.gTxt('are_you_sure').'\')">',
+			echo n.n.'<form action="index.php" id="links_form" class="multi_edit_form" method="post" name="longform">',

 				n.'<div class="txp-listtables">'.
 				n.startTable('', '', 'txp-list').
Index: textpattern.js
===================================================================
--- textpattern.js	(revision 3871)
+++ textpattern.js	(working copy)
@@ -160,7 +160,8 @@
 		'submitButton' : '.multi-edit input[type=submit]',
 		'selectAll' : 'input[name=select_all][type=checkbox]',
 		'rowClick' : true,
-		'altClick' : true
+		'altClick' : true,
+		'confirmation' : textpattern.gTxt('are_you_sure')
 	};

 	if ($.type(method) !== 'string')
@@ -445,24 +446,41 @@
 					return private;
 				}

-				form.button.show();
-
 				if (selected.data('method'))
 				{
 					$(this).after($('<div />').attr('class', 'multi-step multi-option').html(selected.data('method')));
+					form.button.show();
 				}
 				else 
 				{
+					form.button.hide();
 					$(this).parents('form').submit();
 				}
 			});

 			return private;
 		};
-		
+
+		/**
+		 * Handles sending
+		 */
+
+		private.sendForm = function()
+		{
+			$this.submit(function() {
+				if (opt.confirmation !== false && verify(opt.confirmation) === false)
+				{
+					form.editMethod.val('').change();
+					return false;
+				}
+			});
+
+			return private;
+		};
+
 		if(!$this.data('_txpMultiEdit'))
 		{
-			private.bindRows().highlight().extendedClick().checked().changeMethod();
+			private.bindRows().highlight().extendedClick().checked().changeMethod().sendForm();

 			$this.find('.multi-option:not(.multi-step)').each(function() {
 				public.addOption({
Index: lib/txplib_head.php
===================================================================
--- lib/txplib_head.php	(revision 3871)
+++ lib/txplib_head.php	(working copy)
@@ -72,7 +72,7 @@
 													'#page-link #link-title, #page-link #link-description')
 							).'"};'
 	);
-	gTxtScript('form_submission_error');
+	gTxtScript(array('form_submission_error', 'are_you_sure'));
 	?>
 	<script type="text/javascript" src="textpattern.js"></script>
 	<script type="text/javascript">

Bloke wrote:

BUT from a UX perspective it’s probably terrible (just guessing) because then some options will trigger it and some won’t.

Yeah, that might make you expect a confirmation dialog which you then won’t get. Must say that I’ve been victim of that in some other system. Not fun when you end up to reset passwords by accident :/

Last edited by Gocom (2012-06-29 20:14:48)

Offline

#74 2012-06-29 20:17:13

Gocom
Developer Emeritus
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: New sections tab, multi-edit control block

Oops, totally forgot about Forms and Category panels. I’ve updated the (above) patch to include those two too.

Offline

#75 2012-06-29 20:33:02

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 11,273
Website GitHub

Re: New sections tab, multi-edit control block

Wicked stuff. I’ll check it out. Can’t believe I left out the document.ready in the Category & Forms panels. Tut tut.


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

#76 2012-06-29 21:22:43

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 11,273
Website GitHub

Re: New sections tab, multi-edit control block

That’s much better functionality. The ‘reset-on-cancel’ is far superior to leaving it selected and adding the ‘Go’ button for auto-submit actions. Me likey. Me committee…


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

#77 2012-06-29 21:35:44

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 11,273
Website GitHub

Re: New sections tab, multi-edit control block

Oh, one thing. In FF (and I assume other browsers), if you:

  • Select some stuff.
  • Choose an auto-submit action such as ‘delete’, or anything from the Comments panel.
  • Hit Cancel in the verify box.
  • Choose another (or the same) auto-submit action.

you see an additional checkbox in the verify message:

Prevent this page from creating additional dialogs?

Thus it’s tripping the browser’s ‘infinite loop’ dialog security feature.

If anyone does check the box and hits Cancel again, the next time they choose any auto-submit option, no dialog will appear and, further, won’t be able to submit the form because the ‘Go’ button doesn’t appear on auto-submit actions. The only way to restore functionality is to reload the panel.

This behaviour is not a show stopper because the number of times this is going to happen is verging on zero, but thought I’d just document it here so we can refer to it in future in case it crops up.

Last edited by Bloke (2012-06-29 21:36:36)


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

#78 2012-06-29 22:41:03

Gocom
Developer Emeritus
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: New sections tab, multi-edit control block

Bloke wrote:

This behaviour is not a show stopper because the number of times this is going to happen is verging on zero, but thought I’d just document it here so we can refer to it in future in case it crops up.

One of those reasons why there should be a better dialog implementation than relying to confirm() in place. I’m hoping that one comes with the future modal dialog code (in 4.6 maybe?).

In FF (and I assume other browsers)

In Safari and Chrome such check isn’t present in as strict form (at least on Mac OS). I suppose that in IE it’s possible to disable confirm dialogs altogether with the browser’s security options.

Last edited by Gocom (2012-06-29 22:57:30)

Offline

#79 2012-06-30 01:40:04

phiw13
Plugin Author
From: Japan
Registered: 2004-02-27
Posts: 3,081
Website

Re: New sections tab, multi-edit control block

Another bug. Seen this in 3 different browsers, two installs (one at r3862 – pretty much default, the other one at r3875 – my usual test install):

  1. Section pane, select a section,
  2. from the drop down: choose ‘Uses Page’
  3. from the secondary dropdown choose a page template (doesn’t matter which one)
  4. go, confirm (ok), etc

Result: the selected section now has a template named ‘1’ (in the table). Click on the link to that template and you get a blank template page… (that is expected, of course).

Now click on the name of the section you just edited, the edit pane comes up and under ‘Uses Page’ is listed the template it originally had.

This seems specific to changing the ‘Uses Page’ action, and specific to the Sections pane; I haven’t seen any such problems on other panes, or with different actions on the Sections pane.


Where is that emoji for a solar powered submarine when you need it ?
Sand space – admin theme for Textpattern

Offline

#80 2012-06-30 08:31:07

Gocom
Developer Emeritus
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: New sections tab, multi-edit control block

Ah, it’s the input name, page. The multi-edit select for Uses Page option is named page, but it is reserved/used by pagination too, and the edited section gets a page number instead of page template. The multi-edit option’s name needs to be changed. Might not be a bad idea also to verify the values.

Last edited by Gocom (2012-06-30 08:32:54)

Offline

#81 2012-06-30 08:56:49

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 11,273
Website GitHub

Re: New sections tab, multi-edit control block

Gocom wrote:

Might not be a bad idea also to verify the values.

Ack, an extra query, but I suppose you’re right and it’s only on save so the cost isn’t massive. Multi-edit values should probably be checked across the interface actually…

Last edited by Bloke (2012-06-30 08:57:29)


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

#82 2012-06-30 09:00:20

Gocom
Developer Emeritus
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: New sections tab, multi-edit control block

Shouldn’t those values used in SQL queries be escaped?

Index: include/txp_section.php
===================================================================
--- include/txp_section.php	(revision 3877)
+++ include/txp_section.php	(working copy)
@@ -512,7 +512,7 @@
 	function section_delete()
 	{
 		$selected  = ps('selected');
-		$with_articles = safe_rows('Section, Count(*) AS count', 'textpattern', "Section in ('".join("','", $selected)."') GROUP BY Section");
+		$with_articles = safe_rows('Section, Count(*) AS count', 'textpattern', "Section in ('".join("','", doSlash($selected))."') GROUP BY Section");
 		$protected[] = 'default';
 		$del['success'] = $del['error'] = array();

@@ -601,7 +601,7 @@

 		switch ($method)
 		{
-			case 'delete';
+			case 'delete':
 				return section_delete($selected);
 				break;

@@ -610,24 +610,24 @@
 				$val = ps('uses_page');
 				break;

-			case 'changecss';
+			case 'changecss':
 				$key = 'css';
 				$val = ps('css');
 				break;

-			case 'changeonfrontpage';
+			case 'changeonfrontpage':
 				$key = 'on_frontpage';
-				$val = ps('on_front_page');
+				$val = (int) ps('on_front_page');
 				break;

-			case 'changesyndicate';
+			case 'changesyndicate':
 				$key = 'in_rss';
-				$val = ps('in_rss');
+				$val = (int) ps('in_rss');
 				break;

-			case 'changesearchable';
+			case 'changesearchable':
 				$key = 'searchable';
-				$val = ps('searchable');
+				$val = (int) ps('searchable');
 				break;

 			default:
@@ -636,13 +636,13 @@
 				break;
 		}

-		$selected = safe_column('name', 'txp_section', "name IN ('".join("','", $selected)."')");
+		$selected = safe_column('name', 'txp_section', "name IN ('".join("','", doSlash($selected))."')");

 		if ($selected and $key)
 		{
 			foreach ($selected as $id)
 			{
-				if (safe_update('txp_section', "$key = '".doSlash($val)."'", "name = '$id'"))
+				if (safe_update('txp_section', "$key = '".doSlash($val)."'", "name = '".doSlash($id)."'"))
 				{
 					$changed[] = $id;
 				}

Offline

#83 2012-06-30 09:01:43

Gocom
Developer Emeritus
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: New sections tab, multi-edit control block

Bloke wrote:

Ack, an extra query, but I suppose you’re right and it’s only on save so the cost isn’t massive. Multi-edit values should probably be checked across the interface actually…

You could do two queries globally right before $step(); is called and store the values. No extra queries and txp_css and txp_pages tables are accessed on all pages anyways.

Last edited by Gocom (2012-06-30 09:06:25)

Offline

#84 2012-06-30 14:22:30

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 11,273
Website GitHub

Re: New sections tab, multi-edit control block

Gocom wrote:

You could do two queries globally right before $step(); is called and store the values.

True. Or what about a Constraint? Or is this not an appropriate place for it?

Shouldn’t those values used in SQL queries be escaped?

Hell yes! And how did the case statements ever work with semicolons instead of colons?! I need my eyes testing. It’s like that in a few panels so I’d better fix that sharpish. Thanks eagle-eyes.

Last edited by Bloke (2012-06-30 14:22:55)


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Offline

Board footer

Powered by FluxBB