perf(scripts): Load all scripts in foot regardless of registered loca…

…tion BREAKING CHANGE: If you use any inline scripts that depend on scripts in head, you'll need to change them to external AMD modules and load them with `elgg_require_js`. Fixes Elgg#2718
mrclay · May 2, 2015 · c063302 · c063302
1 parent 9db4b55
commit c063302
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 20 deletions.
diff --git a/docs/guides/javascript.rst b/docs/guides/javascript.rst
@@ -122,12 +122,13 @@ Load a library on the current page with ``elgg_load_js``:
 
 This will include and execute the linked code.
 
-.. tip::
+.. warning::
 
-   Using inline scripts is strongly discouraged because:
+   Using inline scripts is NOT SUPPORTED because:
     * They are not testable (maintainability)
     * They are not cacheable (performance)
-    * Doing so forces some scripts to be loaded in ``<head>`` (performance)
+    * They prevent use of Content-Security-Policy (security) 
+    * They prevent scripts from being loaded with ``defer`` or ``async`` (performance)
 
    Inline scripts in core or bundled plugins are considered legacy bugs.
 

diff --git a/docs/guides/upgrading.rst b/docs/guides/upgrading.rst
@@ -12,14 +12,21 @@ See the administrator guides for :doc:`how to upgrade a live site </admin/upgrad
 From 1.11 to 2.0
 ================
 
-Removed Functions and Scripts
------------------------------
+All scripts moved to bottom of page
+-----------------------------------
+
+All inline scripts must be converted to :ref:`AMD <guides/javascript>` or to external scripts loaded with
+``elgg_load_js``. For performance reasons, Elgg no longer loads its core scripts in the ``head`` element,
+and ``elgg_register_js`` no longer honors ``$location == 'head'``, instead outputting all scripts at the
+end of the ``body`` element.
+
+Removed Functions
+-----------------
 
  - get_db_error()
  - execute_delayed_query()
  - get_db_link()
  - load_plugins()
- - mod/groups/icon.php
 
 Callbacks in Queries
 --------------------

diff --git a/engine/lib/elgglib.php b/engine/lib/elgglib.php
@@ -116,6 +116,9 @@ function forward($location = "", $reason = 'system') {
  * The JavaScript files can be local to the server or remote (such as
  * Google's CDN).
  *
+ * @note Since 2.0, scripts with location "head" will also be output in the footer, but before
+ *       those with location "footer".
+ *
  * @param string $name     An identifier for the JavaScript library
  * @param string $url      URL of the JavaScript file
  * @param string $location Page location: head or footer. (default: head)

diff --git a/views/default/page/elements/foot.php b/views/default/page/elements/foot.php
@@ -2,14 +2,20 @@
 
 echo elgg_view_deprecated('footer/analytics', array(), "Extend page/elements/foot instead", 1.8);
 
+$elgg_init = elgg_view('js/initialize_elgg');
+echo "<script>$elgg_init</script>";
+
+// TODO(evan): "head" JS and "footer" JS distinction doesn't make sense anymore
+// TODO(evan): Introduce new "async" location for scripts allowed in head?
+$js = elgg_get_loaded_js('head');
+foreach ($js as $url) {
+	echo elgg_format_element('script', array('src' => $url));
+}
+
 $js = elgg_get_loaded_js('footer');
-foreach ($js as $script) { ?>
-	<script src="<?php echo htmlspecialchars($script, ENT_QUOTES, 'UTF-8'); ?>"></script>
-<?php
+foreach ($js as $url) {
+	echo elgg_format_element('script', array('src' => $url));
 }
 
-$deps = _elgg_services()->amdConfig->getDependencies();
-?>
-<script>
-require(<?php echo json_encode($deps); ?>);
-</script>
+$requires = json_encode(_elgg_services()->amdConfig->getDependencies());
+echo "<script>require($requires)</script>";
diff --git a/views/default/page/elements/head.php b/views/default/page/elements/head.php
@@ -28,9 +28,7 @@
 	echo elgg_format_element('link', $attributes);
 }
 
-$js = elgg_get_loaded_js('head');
 $css = elgg_get_loaded_css();
-$elgg_init = elgg_view('js/initialize_elgg');
 
 $html5shiv_url = elgg_normalize_url('vendors/html5shiv.js');
 $ie_url = elgg_get_simplecache_url('css', 'ie');
@@ -52,11 +50,7 @@
 		<link rel="stylesheet" href="<?php echo $ie_url; ?>" />
 	<![endif]-->
 
-	<script><?php echo $elgg_init; ?></script>
 <?php
-foreach ($js as $url) {
-	echo elgg_format_element('script', array('src' => $url));
-}
 
 echo elgg_view_deprecated('page/elements/shortcut_icon', array(), "Use the 'head', 'page' plugin hook.", 1.9);