Merge branch 'MDL-34182-26' of git://github.com/andrewnicols/moodle into MOODLE_26_STABLE

lib/ajax/ajaxlib.php

@@ -67,3 +67,40 @@ function ajaxenabled(array $browsers = null) {
         return false;
     }
 }
+
+/**
+ * Starts capturing output whilst processing an AJAX request.
+ *
+ * This should be used in combination with ajax_check_captured_output to
+ * report any captured output to the user.
+ *
+ * @retrun Boolean Returns true on success or false on failure.
+ */
+function ajax_capture_output() {
+    // Start capturing output in case of broken plugins.
+    return ob_start();
+}
+
+/**
+ * Check captured output for content. If the site has a debug level of
+ * debugdeveloper set, and the content is non-empty, then throw a coding
+ * exception which can be captured by the Y.IO request and displayed to the
+ * user.
+ *
+ * @return Any output that was captured.
+ */
+function ajax_check_captured_output() {
+    global $CFG;
+
+    // Retrieve the output - there should be none.
+    $output = ob_get_contents();
+    ob_end_clean();
+
+    if ($CFG->debugdeveloper && !empty($output)) {
+        // Only throw an error if the site is in debugdeveloper.
+        throw new coding_exception('Unexpected output whilst processing AJAX request. ' .
+                'This could be caused by trailing whitespace. Output received: ' .
+                var_export($output, true));
+    }
+    return $output;
+}

lib/ajax/getsiteadminbranch.php

@@ -40,7 +40,7 @@ if ($branchtype !== navigation_node::TYPE_SITE_ADMIN) {
 }
 
 // Start capturing output in case of broken plugins.
-ob_start();
+ajax_capture_output();
 
 $PAGE->set_context(context_system::instance());
 $PAGE->set_url('/lib/ajax/getsiteadminbranch.php', array('type'=>$branchtype));
@@ -51,12 +51,5 @@ $sitenavigation = new settings_navigation_ajax($PAGE);
 $converter = new navigation_json();
 $branch = $sitenavigation->get('root');
 
-$output = ob_get_contents();
+ajax_check_captured_output();
-ob_end_clean();
+echo $converter->convert($branch);
-if ($CFG->debugdeveloper && !empty($output)) {
-    throw new coding_exception('Unexpected output whilst building the administration tree. ' .
-            'This could be caused by trailing whitespace. Output received: ' .
-            var_export($output, true));
-} else {
-    echo $converter->convert($branch);
-}

lib/tests/ajaxlib_test.php

@@ -0,0 +1,113 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Code quality unit tests that are fast enough to run each time.
+ *
+ * @package    core
+ * @category   phpunit
+ * @copyright  2013 Andrew Nicols
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU Public License
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+class core_ajaxlib_testcase extends advanced_testcase {
+
+    protected function helper_test_clean_output() {
+        $this->resetAfterTest();
+
+        $result = ajax_capture_output();
+
+        // ob_start should normally return without issue.
+        $this->assertTrue($result);
+
+        $result = ajax_check_captured_output();
+        $this->assertEmpty($result);
+    }
+
+    protected function helper_test_dirty_output($expectexception = false) {
+        $this->resetAfterTest();
+
+        // Keep track of the content we will output.
+        $content = "Some example content";
+
+        $result = ajax_capture_output();
+
+        // ob_start should normally return without issue.
+        $this->assertTrue($result);
+
+        // Fill the output buffer.
+        echo $content;
+
+        if ($expectexception) {
+            $this->setExpectedException('coding_exception');
+            ajax_check_captured_output();
+        } else {
+            $result = ajax_check_captured_output();
+            $this->assertEquals($result, $content);
+        }
+    }
+
+    public function test_output_capture_normal_debug_none() {
+        // In normal conditions, and with DEBUG_NONE set, we should not receive any output or throw any exceptions.
+        set_debugging(DEBUG_NONE);
+        $this->helper_test_clean_output();
+    }
+
+    public function test_output_capture_normal_debug_normal() {
+        // In normal conditions, and with DEBUG_NORMAL set, we should not receive any output or throw any exceptions.
+        set_debugging(DEBUG_NORMAL);
+        $this->helper_test_clean_output();
+    }
+
+    public function test_output_capture_normal_debug_all() {
+        // In normal conditions, and with DEBUG_ALL set, we should not receive any output or throw any exceptions.
+        set_debugging(DEBUG_ALL);
+        $this->helper_test_clean_output();
+    }
+
+    public function test_output_capture_normal_debugdeveloper() {
+        // In normal conditions, and with DEBUG_DEVELOPER set, we should not receive any output or throw any exceptions.
+        set_debugging(DEBUG_DEVELOPER);
+        $this->helper_test_clean_output();
+    }
+
+    public function test_output_capture_error_debug_none() {
+        // With DEBUG_NONE set, we should not throw any exception, but the output will be returned.
+        set_debugging(DEBUG_NONE);
+        $this->helper_test_dirty_output();
+    }
+
+    public function test_output_capture_error_debug_normal() {
+        // With DEBUG_NORMAL set, we should not throw any exception, but the output will be returned.
+        set_debugging(DEBUG_NORMAL);
+        $this->helper_test_dirty_output();
+    }
+
+    public function test_output_capture_error_debug_all() {
+        // In error conditions, and with DEBUG_ALL set, we should not receive any output or throw any exceptions.
+        set_debugging(DEBUG_ALL);
+        $this->helper_test_dirty_output();
+    }
+
+    public function test_output_capture_error_debugdeveloper() {
+        // With DEBUG_DEVELOPER set, we should throw an exception.
+        set_debugging(DEBUG_DEVELOPER);
+        $this->helper_test_dirty_output(true);
+    }
+
+}

repository/filepicker.js

@@ -582,27 +582,23 @@ M.core_filepicker.init = function(Y, options) {
                 method: 'POST',
                 on: {
                     complete: function(id,o,p) {
-                        if (!o) {
-                            // TODO
-                            alert('IO FATAL');
-                            return;
-                        }
                         var data = null;
                         try {
                             data = Y.JSON.parse(o.responseText);
                         } catch(e) {
-                            scope.print_msg(M.str.repository.invalidjson, 'error');
+                            if (o && o.status && o.status > 0) {
-                            scope.display_error(M.str.repository.invalidjson+'<pre>'+stripHTML(o.responseText)+'</pre>', 'invalidjson')
+                                Y.use('moodle-core-notification-exception', function() {
-                            return;
+                                    return new M.core.exception(e);
+                                });
+                                return;
+                            }
                         }
                         // error checking
                         if (data && data.error) {
-                            scope.print_msg(data.error, 'error');
+                            Y.use('moodle-core-notification-ajaxException', function () {
-                            if (args.onerror) {
+                                return new M.core.ajaxException(data);
-                                args.onerror(id,data,p);
+                            });
-                            } else {
+                            this.fpnode.one('.fp-content').setContent('');
-                                this.fpnode.one('.fp-content').setContent('');
-                            }
                             return;
                         } else {
                             if (data.msg) {

repository/repository_ajax.php

@@ -74,6 +74,8 @@ $repooptions = array(
     'ajax' => true,
     'mimetypes' => $accepted_types
 );
+
+ajax_capture_output();
 $repo = repository::get_repository_by_id($repo_id, $contextid, $repooptions);
 
 // Check permissions
@@ -97,6 +99,7 @@ switch ($action) {
         if ($repo->check_login()) {
             $listing = repository::prepare_listing($repo->get_listing($req_path, $page));
             $listing['repo_id'] = $repo_id;
+            ajax_check_captured_output();
             echo json_encode($listing);
             break;
         } else {
@@ -105,23 +108,27 @@ switch ($action) {
     case 'login':
         $listing = $repo->print_login();
         $listing['repo_id'] = $repo_id;
+        ajax_check_captured_output();
         echo json_encode($listing);
         break;
     case 'logout':
         $logout = $repo->logout();
         $logout['repo_id'] = $repo_id;
+        ajax_check_captured_output();
         echo json_encode($logout);
         break;
     case 'searchform':
         $search_form['repo_id'] = $repo_id;
         $search_form['form'] = $repo->print_search();
         $search_form['allowcaching'] = true;
+        ajax_check_captured_output();
         echo json_encode($search_form);
         break;
     case 'search':
         $search_result = repository::prepare_listing($repo->search($search_text, (int)$page));
         $search_result['repo_id'] = $repo_id;
         $search_result['issearchresult'] = true;
+        ajax_check_captured_output();
         echo json_encode($search_result);
         break;
     case 'download':
@@ -160,6 +167,7 @@ switch ($action) {
             $info['file'] = $saveas_filename;
             $info['type'] = 'link';
             $info['url'] = $link;
+            ajax_check_captured_output();
             echo json_encode($info);
             die;
         } else {
@@ -251,6 +259,7 @@ switch ($action) {
                 // You can cache reository file in this callback
                 // or complete other tasks.
                 $repo->cache_file_by_reference($reference, $storedfile);
+                ajax_check_captured_output();
                 echo json_encode($event);
                 die;
             } else if ($repo->has_moodle_files()) {
@@ -261,6 +270,7 @@ switch ($action) {
                 // {@link repository::copy_to_area()}.
                 $fileinfo = $repo->copy_to_area($reference, $record, $maxbytes, $areamaxbytes);
 
+                ajax_check_captured_output();
                 echo json_encode($fileinfo);
                 die;
             } else {
@@ -286,12 +296,14 @@ switch ($action) {
                     $info['e'] = get_string('error', 'moodle');
                 }
             }
+            ajax_check_captured_output();
             echo json_encode($info);
             die;
         }
         break;
     case 'upload':
         $result = $repo->upload($saveas_filename, $maxbytes);
+        ajax_check_captured_output();
         echo json_encode($result);
         break;
 
@@ -304,6 +316,7 @@ switch ($action) {
         $newfilename = required_param('newfilename', PARAM_FILE);
 
         $info = repository::overwrite_existing_draftfile($itemid, $filepath, $filename, $newfilepath, $newfilename);
+        ajax_check_captured_output();
         echo json_encode($info);
         break;
 
@@ -311,6 +324,7 @@ switch ($action) {
         // delete tmp file
         $newfilepath = required_param('newfilepath', PARAM_PATH);
         $newfilename = required_param('newfilename', PARAM_FILE);
+        ajax_check_captured_output();
         echo json_encode(repository::delete_tempfile_from_draft($itemid, $newfilepath, $newfilename));
 
         break;