core/core.libraries.yml | 15 ++++++- core/lib/Drupal/Core/Ajax/AjaxResponse.php | 11 +++++- core/lib/Drupal/Core/Asset/AssetResolver.php | 38 +++++++++++++++--- .../Drupal/Core/Asset/LibraryDiscoveryParser.php | 5 ++- core/modules/editor/editor.libraries.yml | 2 +- core/modules/history/history.libraries.yml | 2 +- core/modules/quickedit/quickedit.libraries.yml | 34 ++++++++-------- core/modules/statistics/statistics.libraries.yml | 2 +- .../system/src/Tests/Common/AttachedAssetsTest.php | 46 +++++++++++----------- .../modules/ajax_test/ajax_test.libraries.yml | 15 ++++--- .../modules/common_test/common_test.libraries.yml | 7 +++- 11 files changed, 118 insertions(+), 59 deletions(-) diff --git a/core/core.libraries.yml b/core/core.libraries.yml index ffa0073..59f4d28 100644 --- a/core/core.libraries.yml +++ b/core/core.libraries.yml @@ -1,5 +1,13 @@ # All libraries are defined in alphabetical order. +# Virtual library, to declare a dependency on the of the HTML document. +# Libraries can declare a dependency on core/ if they wish their JS assets +# to be declared in the HTML . This allows JS that *should* block the page +# from being loaded to be marked as such, but should be used very sparingly: it +# should only be used for absolutely critical UI elements. +: + version: VERSION + backbone: remote: https://github.com/jashkenas/backbone version: 1.1.2 @@ -191,6 +199,8 @@ drupal.dropbutton: - core/drupal - core/drupalSettings - core/jquery.once + # Block the page from being loaded until dropbuttons are initialized. + - core/ drupal.form: version: VERSION @@ -814,7 +824,10 @@ modernizr: gpl-compatible: true version: v2.8.3 js: - assets/vendor/modernizr/modernizr.min.js: { every_page: true, preprocess: 0, scope: header, weight: -21, minified: true } + assets/vendor/modernizr/modernizr.min.js: { every_page: true, preprocess: 0, weight: -21, minified: true } + dependencies: + # Block the page from being loaded until Modernizr is initialized. + - core/ normalize: remote: https://github.com/necolas/normalize.css diff --git a/core/lib/Drupal/Core/Ajax/AjaxResponse.php b/core/lib/Drupal/Core/Ajax/AjaxResponse.php index fcb45af..13b827b 100644 --- a/core/lib/Drupal/Core/Ajax/AjaxResponse.php +++ b/core/lib/Drupal/Core/Ajax/AjaxResponse.php @@ -135,8 +135,15 @@ protected function ajaxRender(Request $request) { // Render the HTML to load these files, and add AJAX commands to insert this // HTML in the page. Settings are handled separately, afterwards. - $settings = (isset($js_assets_header['drupalSettings'])) ? $js_assets_header['drupalSettings']['data'] : []; - unset($js_assets_header['drupalSettings']); + $settings = []; + if (isset($js_assets_header['drupalSettings'])) { + $settings = $js_assets_header['drupalSettings']['data']; + unset($js_assets_header['drupalSettings']); + } + if (isset($js_assets_footer['drupalSettings'])) { + $settings = $js_assets_footer['drupalSettings']['data']; + unset($js_assets_footer['drupalSettings']); + } // Prepend commands to add the assets, preserving their relative order. $resource_commands = array(); diff --git a/core/lib/Drupal/Core/Asset/AssetResolver.php b/core/lib/Drupal/Core/Asset/AssetResolver.php index 1b02761..e8e34452 100644 --- a/core/lib/Drupal/Core/Asset/AssetResolver.php +++ b/core/lib/Drupal/Core/Asset/AssetResolver.php @@ -214,8 +214,27 @@ protected function getJsSettingsAssets(AttachedAssetsInterface $assets) { */ public function getJsAssets(AttachedAssetsInterface $assets, $optimize) { $javascript = []; + $libraries_to_load = $this->getLibrariesToLoad($assets); - foreach ($this->getLibrariesToLoad($assets) as $library) { + // If the core/ library is absent, we know for certain that we have no + // JS assets that must be loaded from the HTML . + $header_js_libraries = []; + if (in_array('core/', $libraries_to_load)) { + // Collect all libraries that contain JS assets and depend on core/. + foreach ($libraries_to_load as $library) { + list($extension, $name) = explode('/', $library, 2); + $definition = $this->libraryDiscovery->getLibraryByName($extension, $name); + if (isset($definition['js']) && in_array('core/', $definition['dependencies'])) { + $header_js_libraries[] = $library; + } + } + // The current list of header JS libraries are only those libraries that + // depend directly on core/, but their dependencies must also be + // loaded for them to function correctly, so update the list with those. + $header_js_libraries = $this->libraryDependencyResolver->getLibrariesWithDependencies($header_js_libraries); + } + + foreach ($libraries_to_load as $library) { list($extension, $name) = explode('/', $library, 2); $definition = $this->libraryDiscovery->getLibraryByName($extension, $name); if (isset($definition['js'])) { @@ -225,7 +244,6 @@ public function getJsAssets(AttachedAssetsInterface $assets, $optimize) { 'group' => JS_DEFAULT, 'every_page' => FALSE, 'weight' => 0, - 'scope' => 'header', 'cache' => TRUE, 'preprocess' => TRUE, 'attributes' => array(), @@ -233,6 +251,9 @@ public function getJsAssets(AttachedAssetsInterface $assets, $optimize) { 'browsers' => array(), ); + // 'scope' is a calculated option, based on dependencies (see above). + $options['scope'] = in_array($library, $header_js_libraries) ? 'header' : 'footer'; + // Preprocess can only be set if caching is enabled and no attributes // are set. $options['preprocess'] = $options['cache'] && empty($options['attributes']) ? $options['preprocess'] : FALSE; @@ -267,8 +288,6 @@ public function getJsAssets(AttachedAssetsInterface $assets, $optimize) { } } - // @todo Refactor this when the default scope is changed to 'footer' in - // https://www.drupal.org/node/784626 // If the core/drupalSettings library is being loaded or is already loaded, // get the JavaScript settings assets, and convert them into a single // "regular" JavaScript asset. @@ -278,7 +297,6 @@ public function getJsAssets(AttachedAssetsInterface $assets, $optimize) { if ($settings_needed && $settings_have_changed) { $settings = $this->getJsSettingsAssets($assets); if (!empty($settings)) { - // Prepend to the list of JavaScript assets, to render it first. $settings_as_inline_javascript = [ 'type' => 'setting', 'group' => JS_SETTING, @@ -287,7 +305,15 @@ public function getJsAssets(AttachedAssetsInterface $assets, $optimize) { 'browsers' => array(), 'data' => $settings, ]; - $js_assets_header = ['drupalSettings' => $settings_as_inline_javascript] + $js_assets_header; + $settings_js_asset = ['drupalSettings' => $settings_as_inline_javascript]; + // Prepend to the list of JS assets, to render it first. Preferably in + // the footer, but in the header if necessary. + if (in_array('core/drupalSettings', $header_js_libraries)) { + $js_assets_header = $settings_js_asset + $js_assets_header; + } + else { + $js_assets_footer = $settings_js_asset + $js_assets_footer; + } } } diff --git a/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php index 5ca5f26..f856de9 100644 --- a/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php +++ b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php @@ -82,7 +82,10 @@ public function buildByExtension($extension) { foreach ($libraries as $id => &$library) { if (!isset($library['js']) && !isset($library['css']) && !isset($library['drupalSettings'])) { - throw new IncompleteLibraryDefinitionException(sprintf("Incomplete library definition for definition '%s' in extension '%s'", $id, $extension)); + // The core/ library is the only allowed 'virtual' library. + if ($extension !== 'core' || $id !== '') { + throw new IncompleteLibraryDefinitionException(sprintf("Incomplete library definition for definition '%s' in extension '%s'", $id, $extension)); + } } $library += array('dependencies' => array(), 'js' => array(), 'css' => array()); diff --git a/core/modules/editor/editor.libraries.yml b/core/modules/editor/editor.libraries.yml index 8a95be3..d59d259 100644 --- a/core/modules/editor/editor.libraries.yml +++ b/core/modules/editor/editor.libraries.yml @@ -34,7 +34,7 @@ drupal.editor.dialog: quickedit.inPlaceEditor.formattedText: version: VERSION js: - js/editor.formattedTextEditor.js: { scope: footer, attributes: { defer: true } } + js/editor.formattedTextEditor.js: { attributes: { defer: true } } dependencies: - quickedit/quickedit - editor/drupal.editor diff --git a/core/modules/history/history.libraries.yml b/core/modules/history/history.libraries.yml index 457b1f4..97a8418 100644 --- a/core/modules/history/history.libraries.yml +++ b/core/modules/history/history.libraries.yml @@ -11,6 +11,6 @@ api: mark-as-read: version: VERSION js: - js/mark-as-read.js: { scope: footer } + js/mark-as-read.js: {} dependencies: - history/api diff --git a/core/modules/quickedit/quickedit.libraries.yml b/core/modules/quickedit/quickedit.libraries.yml index 74f9c19..184864e 100644 --- a/core/modules/quickedit/quickedit.libraries.yml +++ b/core/modules/quickedit/quickedit.libraries.yml @@ -2,24 +2,24 @@ quickedit: version: VERSION js: # Core. - js/quickedit.js: { scope: footer } - js/util.js: { scope: footer } + js/quickedit.js: {} + js/util.js: {} # Models. - js/models/BaseModel.js: { scope: footer } - js/models/AppModel.js: { scope: footer } - js/models/EntityModel.js: { scope: footer } - js/models/FieldModel.js: { scope: footer } - js/models/EditorModel.js: { scope: footer } + js/models/BaseModel.js: {} + js/models/AppModel.js: {} + js/models/EntityModel.js: {} + js/models/FieldModel.js: {} + js/models/EditorModel.js: {} # Views. - js/views/AppView.js: { scope: footer } - js/views/FieldDecorationView.js: { scope: footer } - js/views/EntityDecorationView.js: { scope: footer } - js/views/EntityToolbarView.js: { scope: footer } - js/views/ContextualLinkView.js: { scope: footer } - js/views/FieldToolbarView.js: { scope: footer } - js/views/EditorView.js: { scope: footer } + js/views/AppView.js: {} + js/views/FieldDecorationView.js: {} + js/views/EntityDecorationView.js: {} + js/views/EntityToolbarView.js: {} + js/views/ContextualLinkView.js: {} + js/views/FieldToolbarView.js: {} + js/views/EditorView.js: {} # Other. - js/theme.js: { scope: footer } + js/theme.js: {} css: component: css/quickedit.module.css: {} @@ -44,13 +44,13 @@ quickedit: quickedit.inPlaceEditor.form: version: VERSION js: - js/editors/formEditor.js: { scope: footer } + js/editors/formEditor.js: {} dependencies: - quickedit/quickedit quickedit.inPlaceEditor.plainText: version: VERSION js: - js/editors/plainTextEditor.js: { scope: footer } + js/editors/plainTextEditor.js: {} dependencies: - quickedit/quickedit diff --git a/core/modules/statistics/statistics.libraries.yml b/core/modules/statistics/statistics.libraries.yml index 2fbe5c0..76c58a1 100644 --- a/core/modules/statistics/statistics.libraries.yml +++ b/core/modules/statistics/statistics.libraries.yml @@ -1,7 +1,7 @@ drupal.statistics: version: VERSION js: - statistics.js: { scope: footer } + statistics.js: {} dependencies: - core/jquery - core/drupal diff --git a/core/modules/system/src/Tests/Common/AttachedAssetsTest.php b/core/modules/system/src/Tests/Common/AttachedAssetsTest.php index 7cd97b3..ac7c47b 100644 --- a/core/modules/system/src/Tests/Common/AttachedAssetsTest.php +++ b/core/modules/system/src/Tests/Common/AttachedAssetsTest.php @@ -86,7 +86,7 @@ function testAddFiles() { $assets = AttachedAssets::createFromRenderArray($build); $css = $this->assetResolver->getCssAssets($assets, FALSE); - $js = $this->assetResolver->getJsAssets($assets, FALSE)[0]; + $js = $this->assetResolver->getJsAssets($assets, FALSE)[1]; $this->assertTrue(array_key_exists('bar.css', $css), 'CSS files are correctly added.'); $this->assertTrue(array_key_exists('core/modules/system/tests/modules/common_test/foo.js', $js), 'JavaScript files are correctly added.'); @@ -107,11 +107,11 @@ function testAddJsSettings() { $build['#attached']['library'][] = 'core/drupalSettings'; $assets = AttachedAssets::createFromRenderArray($build); - $javascript = $this->assetResolver->getJsAssets($assets, FALSE)[0]; + $javascript = $this->assetResolver->getJsAssets($assets, FALSE)[1]; $this->assertTrue(array_key_exists('currentPath', $javascript['drupalSettings']['data']['path']), 'The current path JavaScript setting is set correctly.'); $assets->setSettings(['drupal' => 'rocks', 'dries' => 280342800]); - $javascript = $this->assetResolver->getJsAssets($assets, FALSE)[0]; + $javascript = $this->assetResolver->getJsAssets($assets, FALSE)[1]; $this->assertEqual(280342800, $javascript['drupalSettings']['data']['dries'], 'JavaScript setting is set correctly.'); $this->assertEqual('rocks', $javascript['drupalSettings']['data']['drupal'], 'The other JavaScript setting is set correctly.'); } @@ -124,7 +124,7 @@ function testAddExternalFiles() { $assets = AttachedAssets::createFromRenderArray($build); $css = $this->assetResolver->getCssAssets($assets, FALSE); - $js = $this->assetResolver->getJsAssets($assets, FALSE)[0]; + $js = $this->assetResolver->getJsAssets($assets, FALSE)[1]; $this->assertTrue(array_key_exists('http://example.com/stylesheet.css', $css), 'External CSS files are correctly added.'); $this->assertTrue(array_key_exists('http://example.com/script.js', $js), 'External JavaScript files are correctly added.'); @@ -143,7 +143,7 @@ function testAttributes() { $build['#attached']['library'][] = 'common_test/js-attributes'; $assets = AttachedAssets::createFromRenderArray($build); - $js = $this->assetResolver->getJsAssets($assets, FALSE)[0]; + $js = $this->assetResolver->getJsAssets($assets, FALSE)[1]; $js_render_array = \Drupal::service('asset.js.collection_renderer')->render($js); $rendered_js = $this->renderer->render($js_render_array); $expected_1 = ''; @@ -159,7 +159,7 @@ function testAggregatedAttributes() { $build['#attached']['library'][] = 'common_test/js-attributes'; $assets = AttachedAssets::createFromRenderArray($build); - $js = $this->assetResolver->getJsAssets($assets, TRUE)[0]; + $js = $this->assetResolver->getJsAssets($assets, TRUE)[1]; $js_render_array = \Drupal::service('asset.js.collection_renderer')->render($js); $rendered_js = $this->renderer->render($js_render_array); $expected_1 = ''; @@ -169,16 +169,16 @@ function testAggregatedAttributes() { } /** - * Tests drupal_get_js() for JavaScript settings. + * Tests JavaScript settings. */ - function testHeaderSetting() { + function testSettings() { $build = array(); $build['#attached']['library'][] = 'core/drupalSettings'; // Nonsensical value to verify if it's possible to override path settings. $build['#attached']['drupalSettings']['path']['pathPrefix'] = 'yarhar'; $assets = AttachedAssets::createFromRenderArray($build); - $js = $this->assetResolver->getJsAssets($assets, FALSE)[0]; + $js = $this->assetResolver->getJsAssets($assets, FALSE)[1]; $js_render_array = \Drupal::service('asset.js.collection_renderer')->render($js); $rendered_js = $this->renderer->render($js_render_array); @@ -207,17 +207,19 @@ function testHeaderSetting() { } /** - * Tests JS assets assigned to the 'footer' scope. + * Tests JS assets depending on the 'core/' virtual library. */ - function testFooterHTML() { - $build['#attached']['library'][] = 'common_test/js-footer'; + function testHeaderHTML() { + $build['#attached']['library'][] = 'common_test/js-header'; $assets = AttachedAssets::createFromRenderArray($build); - $js = $this->assetResolver->getJsAssets($assets, FALSE)[1]; + $js = $this->assetResolver->getJsAssets($assets, FALSE)[0]; $js_render_array = \Drupal::service('asset.js.collection_renderer')->render($js); $rendered_js = $this->renderer->render($js_render_array); $query_string = $this->container->get('state')->get('system.css_js_query_string') ?: '0'; - $this->assertNotIdentical(strpos($rendered_js, ''), FALSE, 'Rendering an external JavaScript file.'); + $this->assertNotIdentical(strpos($rendered_js, ''), FALSE, 'The JS asset in common_test/js-header appears in the header.'); + $this->assertNotIdentical(strpos($rendered_js, '