From b92f75996307c169615c8a575f03b7a326b6cdc7 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Thu, 27 Sep 2012 12:14:14 +1200 Subject: [PATCH 1/2] Fixing test to be less fragile (selects the input ID directly instead of holder) --- tests/controller/CMSPageHistoryControllerTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/controller/CMSPageHistoryControllerTest.php b/tests/controller/CMSPageHistoryControllerTest.php index 769b0466..58ec75d6 100755 --- a/tests/controller/CMSPageHistoryControllerTest.php +++ b/tests/controller/CMSPageHistoryControllerTest.php @@ -86,7 +86,7 @@ class CMSPageHistoryControllerTest extends FunctionalTest { */ public function testVersionsForm() { $history = $this->get('admin/pages/history/show/'. $this->page->ID); - $form = $this->cssParser()->getBySelector("#Form_VersionsForm"); + $form = $this->cssParser()->getBySelector('#Form_VersionsForm'); $this->assertEquals(1, count($form)); @@ -103,7 +103,7 @@ class CMSPageHistoryControllerTest extends FunctionalTest { public function testVersionsFormTableContainsInformation() { $history = $this->get('admin/pages/history/show/'. $this->page->ID); - $form = $this->cssParser()->getBySelector("#Form_VersionsForm"); + $form = $this->cssParser()->getBySelector('#Form_VersionsForm'); $rows = $form[0]->xpath("fieldset/table/tbody/tr"); $expected = array( @@ -128,7 +128,7 @@ class CMSPageHistoryControllerTest extends FunctionalTest { public function testVersionsFormSelectsUnpublishedCheckbox() { $history = $this->get('admin/pages/history/show/'. $this->page->ID); - $checkbox = $this->cssParser()->getBySelector("#Form_VersionsForm #ShowUnpublished input"); + $checkbox = $this->cssParser()->getBySelector('#Form_VersionsForm_ShowUnpublished'); $this->assertThat($checkbox[0], $this->logicalNot($this->isNull())); $checked = $checkbox[0]->attributes()->checked; @@ -137,7 +137,7 @@ class CMSPageHistoryControllerTest extends FunctionalTest { // viewing an unpublished $history = $this->get('admin/pages/history/show/'.$this->page->ID .'/'.$this->versionUnpublishedCheck); - $checkbox = $this->cssParser()->getBySelector("#Form_VersionsForm #ShowUnpublished input"); + $checkbox = $this->cssParser()->getBySelector('#Form_VersionsForm_ShowUnpublished'); $this->assertThat($checkbox[0], $this->logicalNot($this->isNull())); $this->assertEquals('checked', (string) $checkbox[0]->attributes()->checked); From 39792debb8888237639d2b316181e685658894d8 Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Thu, 27 Sep 2012 12:30:44 +1200 Subject: [PATCH 2/2] FIX Use RequestHandler::httpError() for all HTTP errors. https://github.com/silverstripe/sapphire/pull/827 adds some extension points for catching HTTP errors such as 404. This change fixes some issues where httpError() isn't used all the time. Note that the aforementioned pull request will be necessary to ensure that it works properly. --- code/controllers/ContentController.php | 9 ++++----- code/controllers/ModelAsController.php | 7 ++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/code/controllers/ContentController.php b/code/controllers/ContentController.php index 5f67e6b7..4593d302 100644 --- a/code/controllers/ContentController.php +++ b/code/controllers/ContentController.php @@ -205,11 +205,10 @@ class ContentController extends Controller { * @uses ErrorPage::response_for() */ public function httpError($code, $message = null) { - if($this->request->isMedia() || !$response = ErrorPage::response_for($code)) { - parent::httpError($code, $message); - } else { - throw new SS_HTTPResponse_Exception($response); - } + // Don't use the HTML response for media requests + $response = $this->request->isMedia() ? null : ErrorPage::response_for($code); + // Failover to $message if the HTML response is unavailable / inappropriate + parent::httpError($code, $response ? $response : $message); } /** diff --git a/code/controllers/ModelAsController.php b/code/controllers/ModelAsController.php index e6d0178b..5aeefc1f 100644 --- a/code/controllers/ModelAsController.php +++ b/code/controllers/ModelAsController.php @@ -126,11 +126,8 @@ class ModelAsController extends Controller implements NestedController { return $this->response; } - if($response = ErrorPage::response_for(404)) { - return $response; - } else { - $this->httpError(404, 'The requested page could not be found.'); - } + $response = ErrorPage::response_for(404); + $this->httpError(404, $response ? $response : 'The requested page could not be found.'); } // Enforce current locale setting to the loaded SiteTree object