From 8fbe46f3e810c361858d6c3b6e2ccbb9ff969ff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Tue, 28 Nov 2017 08:22:46 +0100 Subject: [PATCH 1/4] Remove usage of get_scripts.js.php wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - this adds significant overhead in sending static files from the server - the benefit of joining requests is minimal since introduction of HTTP/2 - the onload handlers are now fired directly by individual scripts, what makes them really execute when the script is loaded (it could be previously executed earlier in case of chunked loading) Fixes #13840 Signed-off-by: Michal Čihař --- js/ajax.js | 65 +++++++++++++++---------------- js/get_scripts.js.php | 72 ----------------------------------- libraries/classes/Scripts.php | 18 +-------- test/classes/ScriptsTest.php | 6 +-- test/libraries/Files_test.php | 1 - 5 files changed, 35 insertions(+), 127 deletions(-) delete mode 100644 js/get_scripts.js.php diff --git a/js/ajax.js b/js/ajax.js index 011980b646..da1e37e892 100644 --- a/js/ajax.js +++ b/js/ajax.js @@ -534,9 +534,10 @@ var AJAX = { _scriptsToBeLoaded: [], /** * @var array _scriptsToBeFired The list of files for which - * to fire the onload event + * to fire the onload and unload events */ _scriptsToBeFired: [], + _scriptsCompleted: false, /** * Records that a file has been downloaded * @@ -572,7 +573,7 @@ var AJAX = { self._scripts = []; self._scriptsVersion = PMA_commonParams.get('PMA_VERSION'); } - self._scriptsToBeLoaded = []; + self._scriptsCompleted = false; self._scriptsToBeFired = []; for (var i in files) { self._scriptsToBeLoaded.push(files[i].name); @@ -580,64 +581,60 @@ var AJAX = { self._scriptsToBeFired.push(files[i].name); } } - // Generate a request string - var request = []; - var needRequest = false; - for (var index in self._scriptsToBeLoaded) { - var script = self._scriptsToBeLoaded[index]; + for (var i in files) { + var script = files[i].name; // Only for scripts that we don't already have if ($.inArray(script, self._scripts) === -1) { - needRequest = true; this.add(script); - request.push('scripts%5B%5D=' + script); - if (request.length >= 10) { - // Download scripts in chunks - this.appendScript(request); - request = []; - needRequest = false; - } + this.appendScript(script, callback); + } else { + self.done(script, callback); } } - request.push('call_done=1'); - request.push('v=' + encodeURIComponent(PMA_commonParams.get('PMA_VERSION'))); - // Download the composite js file, if necessary - if (needRequest) { - this.appendScript(request); - } else { - self.done(callback); - } + // Trigger callback if there is nothing to load + self.done(null, callback); }, /** * Called whenever all files are loaded * * @return void */ - done: function (callback) { - if ($.isFunction(callback)) { - callback(); - } + done: function (script, callback) { if (typeof ErrorReport !== 'undefined') { ErrorReport.wrap_global_functions(); } - for (var i in this._scriptsToBeFired) { - AJAX.fireOnload(this._scriptsToBeFired[i]); + if ($.inArray(script, this._scriptsToBeFired)) { + AJAX.fireOnload(script); + } + if ($.inArray(script, this._scriptsToBeLoaded)) { + this._scriptsToBeLoaded.splice($.inArray(script, this._scriptsToBeLoaded), 1); + } + if (script === null) { + this._scriptsCompleted = true; + } + /* We need to wait for last signal (with null) or last script load */ + AJAX.active = (this._scriptsToBeLoaded.length > 0) || ! this._scriptsCompleted; + /* Run callback on last script */ + if (! AJAX.active && $.isFunction(callback)) { + callback(); } - AJAX.active = false; }, /** * Appends a script element to the head to load the scripts * * @return void */ - appendScript: function (request) { + appendScript: function (name, callback) { var head = document.head || document.getElementsByTagName('head')[0]; var script = document.createElement('script'); + var self = this; - request.push('call_done=1'); - request.push('v=' + encodeURIComponent(PMA_commonParams.get('PMA_VERSION'))); script.type = 'text/javascript'; - script.src = 'js/get_scripts.js.php?' + request.join('&'); + script.src = 'js/' + name + '?' + 'v=' + encodeURIComponent(PMA_commonParams.get('PMA_VERSION')); script.async = false; + script.onload = function () { + self.done(name, callback); + }; head.appendChild(script); }, /** diff --git a/js/get_scripts.js.php b/js/get_scripts.js.php deleted file mode 100644 index 2605ff9f6e..0000000000 --- a/js/get_scripts.js.php +++ /dev/null @@ -1,72 +0,0 @@ -start(); -if (!defined('TESTSUITE')) { - register_shutdown_function( - function () { - echo PhpMyAdmin\OutputBuffering::getInstance()->getContents(); - } - ); -} - -$_GET['scripts'] = json_decode($_GET['scripts']); -if (! empty($_GET['scripts']) && is_array($_GET['scripts'])) { - // Only up to 10 scripts as this is what we generate - foreach (array_slice($_GET['scripts'], 0, 10) as $script) { - // Sanitise filename - $script_name = 'js'; - - $path = explode("/", $script); - foreach ($path as $filename) { - // Allow alphanumeric, "." and "-" chars only, no files starting - // with . - if (preg_match("@^[\w][\w\.-]+$@", $filename)) { - $script_name .= DIRECTORY_SEPARATOR . $filename; - } - } - - // Output file contents - if (preg_match("@\.js$@", $script_name) && is_readable($script_name)) { - readfile($script_name); - echo ";\n\n"; - } - } -} - -if (isset($_GET['call_done'])) { - echo "AJAX.scriptHandler.done();"; -} diff --git a/libraries/classes/Scripts.php b/libraries/classes/Scripts.php index 81bcbbc818..f49bcad14d 100644 --- a/libraries/classes/Scripts.php +++ b/libraries/classes/Scripts.php @@ -61,24 +61,10 @@ class Scripts . "'>"; } } else { - $scripts[] = "scripts%5B%5D=" . $value['filename']; + $result[] = ''; } } - $separator = Url::getArgSeparator(); - // Using chunks of 10 files to avoid too long URLs - // as some servers are set to 512 bytes URL limit - $script_chunks = array_chunk($scripts, 10); - foreach ($script_chunks as $script_chunk) { - $url = 'js/get_scripts.js.php?' - . implode($separator, $script_chunk) - . $separator . Header::getVersionParameter(); - - $result[] = sprintf( - '', - htmlspecialchars($url) - ); - } return implode("\n", $first) . implode("\n", $result); } diff --git a/test/classes/ScriptsTest.php b/test/classes/ScriptsTest.php index 54c196621e..f72c195874 100644 --- a/test/classes/ScriptsTest.php +++ b/test/classes/ScriptsTest.php @@ -77,8 +77,7 @@ class ScriptsTest extends PmaTestCase { $this->assertEquals( '', + . 'src="js/common.js?v=' . PMA_VERSION . '">', $this->_callPrivateFunction( '_includeFiles', array( @@ -105,8 +104,7 @@ class ScriptsTest extends PmaTestCase $this->assertRegExp( '@' + . 'src="js/common.js\?v=' . PMA_VERSION . '">' . '"; - } else { - $result[] = ""; - } + $result .= "\n"; } else { - $result[] = ''; + $result .= '' . "\n"; } } - return implode("\n", $first) . implode("\n", $result); + return $result; } /** @@ -83,15 +74,12 @@ class Scripts * Adds a new file to the list of scripts * * @param string $filename The name of the file to include - * @param bool $before_statics Whether this dynamic script should be - * included before the static ones * @param array $params Additional parameters to pass to the file * * @return void */ public function addFile( $filename, - $before_statics = false, array $params = array() ) { $hash = md5($filename); @@ -104,7 +92,6 @@ class Scripts 'has_onload' => $has_onload, 'filename' => $filename, 'params' => $params, - 'before_statics' => $before_statics ); } diff --git a/test/classes/ScriptsTest.php b/test/classes/ScriptsTest.php index f72c195874..d793c236af 100644 --- a/test/classes/ScriptsTest.php +++ b/test/classes/ScriptsTest.php @@ -77,7 +77,7 @@ class ScriptsTest extends PmaTestCase { $this->assertEquals( '', + . 'src="js/common.js?v=' . PMA_VERSION . '">' . "\n", $this->_callPrivateFunction( '_includeFiles', array( @@ -104,7 +104,7 @@ class ScriptsTest extends PmaTestCase $this->assertRegExp( '@' + . 'src="js/common.js\?v=' . PMA_VERSION . '">' . "\n" . '