From bcbf4636fcb3b225f296754a64d7e1bd25f8994d Mon Sep 17 00:00:00 2001
From: Ingo Schommer <ingo@silverstripe.com>
Date: Wed, 31 Oct 2012 15:57:48 +0100
Subject: [PATCH] BUG Remove .ss-tabset class from CMS tabs to prevent rogue
 ajax load (#7980)

The existence of .ss-tabset triggers JS which applies $.tabs(),
and in turn interprets the first available link as the tab navigation.
jQuery UI subsequently tries to ajax-load this link, which is not
desired. Instead, $.tabs() should *only* be applied to a container
DOM element with .cms-tabset applied.
---
 admin/code/LeftAndMain.php              | 4 +++-
 admin/code/SecurityAdmin.php            | 8 ++++++--
 admin/templates/CMSTabSet.ss            | 8 ++++++--
 forms/TabSet.php                        | 2 +-
 forms/gridfield/GridFieldDetailForm.php | 2 +-
 templates/forms/TabSet.ss               | 2 +-
 tests/forms/FormFieldTest.php           | 2 +-
 7 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/admin/code/LeftAndMain.php b/admin/code/LeftAndMain.php
index c660bae7f..a09d204af 100644
--- a/admin/code/LeftAndMain.php
+++ b/admin/code/LeftAndMain.php
@@ -1091,7 +1091,9 @@ class LeftAndMain extends Controller implements PermissionProvider {
 			$form->setAttribute('data-pjax-fragment', 'CurrentForm');
 			
 			// Set this if you want to split up tabs into a separate header row
-			// if($form->Fields()->hasTabset()) $form->Fields()->findOrMakeTab('Root')->setTemplate('CMSTabSet');
+			// if($form->Fields()->hasTabset()) {
+			// 	$form->Fields()->findOrMakeTab('Root')->setTemplate('CMSTabSet');
+			// }
 			
 			// Add a default or custom validator.
 			// @todo Currently the default Validator.js implementation
diff --git a/admin/code/SecurityAdmin.php b/admin/code/SecurityAdmin.php
index 5ab801373..97be34d53 100755
--- a/admin/code/SecurityAdmin.php
+++ b/admin/code/SecurityAdmin.php
@@ -127,7 +127,8 @@ class SecurityAdmin extends LeftAndMain implements PermissionProvider {
 			// necessary for tree node selection in LeftAndMain.EditForm.js
 			new HiddenField('ID', false, 0)
 		);
-		
+
+		// Tab nav in CMS is rendered through separate template		
 		$root->setTemplate('CMSTabSet');
 		
 		// Add roles editing interface
@@ -161,7 +162,10 @@ class SecurityAdmin extends LeftAndMain implements PermissionProvider {
 		);
 		$form->addExtraClass('cms-edit-form');
 		$form->setTemplate($this->getTemplatesWithSuffix('_EditForm'));
-		if($form->Fields()->hasTabset()) $form->Fields()->findOrMakeTab('Root')->setTemplate('CMSTabSet');
+		// Tab nav in CMS is rendered through separate template
+		if($form->Fields()->hasTabset()) {
+			$form->Fields()->findOrMakeTab('Root')->setTemplate('CMSTabSet');
+		}
 		$form->addExtraClass('center ss-tabset cms-tabset ' . $this->BaseCSSClasses());
 		$form->setAttribute('data-pjax-fragment', 'CurrentForm');
 
diff --git a/admin/templates/CMSTabSet.ss b/admin/templates/CMSTabSet.ss
index 5d4748627..835bdac26 100644
--- a/admin/templates/CMSTabSet.ss
+++ b/admin/templates/CMSTabSet.ss
@@ -1,6 +1,10 @@
-<div $AttributesHTML>
-	<%-- Tab nav is rendered in CMSEditForm.ss --%>
+<%-- Exclude ".ss-tabset" class to avoid inheriting behaviour --%>
+<%-- The ".cms-tabset" class needs to be manually applied to a container elment, --%>
+<%-- above the level where the tab navigation is placed. --%>
+<%-- Tab navigation is rendered through various templates, --%>
+<%-- e.g. through LeftAndMain_EditForm.ss. --%>
 
+<div $AttributesHTML>
 	<% loop Tabs %>
 	<div $AttributesHTML>
 	<% if Tabs %>
diff --git a/forms/TabSet.php b/forms/TabSet.php
index 2966bccda..9d7f73cfb 100644
--- a/forms/TabSet.php
+++ b/forms/TabSet.php
@@ -108,7 +108,7 @@ class TabSet extends CompositeField {
 			$this->attributes,
 			array(
 				'id' => $this->id(),
-				'class' => 'ss-tabset ' . $this->extraClass()
+				'class' => $this->extraClass()
 			)
 		);
 	}
diff --git a/forms/gridfield/GridFieldDetailForm.php b/forms/gridfield/GridFieldDetailForm.php
index a32acbe14..4aeef7931 100644
--- a/forms/gridfield/GridFieldDetailForm.php
+++ b/forms/gridfield/GridFieldDetailForm.php
@@ -340,7 +340,7 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler {
 			$form->setAttribute('data-pjax-fragment', 'CurrentForm Content');
 			if($form->Fields()->hasTabset()) {
 				$form->Fields()->findOrMakeTab('Root')->setTemplate('CMSTabSet');
-				$form->addExtraClass('ss-tabset cms-tabset');
+				$form->addExtraClass('cms-tabset');
 			}
 
 			$form->Backlink = $this->getBackLink();
diff --git a/templates/forms/TabSet.ss b/templates/forms/TabSet.ss
index 77f5f08be..9f1535037 100644
--- a/templates/forms/TabSet.ss
+++ b/templates/forms/TabSet.ss
@@ -1,4 +1,4 @@
-<div $AttributesHTML>
+<div $getAttributesHTML("class") class="ss-tabset $extraClass">
 	<ul>
 	<% loop Tabs %>
 		<li class="$FirstLast $MiddleString $extraClass"><a href="#$id" id="tab-$id">$Title</a></li>
diff --git a/tests/forms/FormFieldTest.php b/tests/forms/FormFieldTest.php
index 6c53eee5b..c7c23645d 100644
--- a/tests/forms/FormFieldTest.php
+++ b/tests/forms/FormFieldTest.php
@@ -64,7 +64,7 @@ class FormFieldTest extends SapphireTest {
 		$this->assertNotContains('two="2"', $field->getAttributesHTML('one', 'two'));
 		$this->assertContains('three="3"', $field->getAttributesHTML('one', 'two'));
 	}
-	
+
 	public function testEveryFieldTransformsReadonlyAsClone() {
 		$fieldClasses = ClassInfo::subclassesFor('FormField');
 		foreach($fieldClasses as $fieldClass) {