Maintaining the Mayo theme for Drupal 8 beta 4 and I get the attached notice and warnings that weren't there in Drupal 8 beta 3. I'm pretty sure that the errors are related to this issue https://www.drupal.org/node/2390707 which removed hook_library_alter(). I reverted the patch in #3 there and the issue is gone. After that I went back to the patched color.module and the issue didn't come back for unknown reasons.

I did separate intall of Drupal 8 alpha 4 and Mayo 8 dev and the errors are there again.

The problem does not arise with Bartik as far as I can tell so contributed themes may have to do some adjusting. The problem seems to come from mayo.libraries.yml and the color.preview library. 

color.preview:
  version: VERSION
  css:
    theme:
      color/preview.css: {}
      css/admin-layout-settings.css: {}
  js:
    color/preview.js: {}
    js/mayo.js: {}
  dependencies:
    - color/drupal.color
    - core/drupalSettings

The library is being called at the bottom of themes/mayo/color/color.inc with 'preview_library' => 'mayo/color.preview',

CommentFileSizeAuthor
#10 undefined_index_css_notice-2404253-10-whole.patch4.14 KBwuinfo - Bill Wu
#10 undefined_index_css_notice-2404253-10-test.patch1.14 KBwuinfo - Bill Wu
#4 undefined_index_css_notice-2404253-4.patch2.99 KBmermentau
warning.png149.57 KBmermentau
notice.png149.24 KBmermentau
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ianthomas_uk’s picture

ianthomas_uk
he/him
Primary language English
Location Brighton, UK
CreditAttribution: ianthomas_uk commented
Category: Bug report » Support request

I can't see anything obviously wrong with either the module or the patch. Have you confirmed exactly which library is causing the problem?

For example, use a debugger or just above line 81 of color.module you could put:

if (!isset($libraries[$name]['css'])) {
print_r($name);
print_r($libraries[$name]);
die;
}

I've set this to a support request until we've confirmed if/what the bug in core actually is.

mermentau’s picture

mermentau CreditAttribution: mermentau commented

The original errors in the attached images indicated the color.preview library, but the debug snip in #1 pointed to the mayo-columns library. The full mayo.libraries.yml file is here:

global-styling:
  version: VERSION
  css:
    theme:
      css/style.css: {}
      css/colors.css: {}

color.preview:
  version: VERSION
  css:
    theme:
      color/preview.css: {}
      css/admin-layout-settings.css: {}
  js:
    color/preview.js: {}
    js/mayo.js: {}
  dependencies:
    - color/drupal.color
    - core/drupalSettings

maintenance-page:
  version: VERSION
  css:
    theme:
      css/maintenance-page.css: {}
      css/colors.css: {}
  dependencies:
    - system/maintenance
    - mayo/global-styling

mayo-columns:
  version: VERSION
  js:
    js/mayo-columns.js: {}
  dependencies:
    - core/jquery
    - core/drupal
    - core/drupalSettings

responsive-layout:
  version: VERSION
  css:
    theme:
      public://mayo/mayo_files/mayo.responsive.layout.css: {}
  dependencies:
    - mayo/global-styling

black-menu:
  version: VERSION
  css:
    theme:
      css/black-menu.css: {}
  dependencies:
    - mayo/global-styling

superfish:
  version: VERSION
  css:
    theme:
      css/mayo-superfish.css: {}
  dependencies:
    - mayo/global-styling

fontsizer:
  version: VERSION
  js:
    js/mayo-fontsize.js: {}
  dependencies:
    - core/jquery

Commenting out the color.preview library causes the error message to point to the global-styling library.
As it stands now the color module copies css/colors.css properly to public:// and everything seems to work except for the nagging "warning" and "notice" messages.

Remember this all works when I role the core patch back so possibly something will have to be adjusted to use the new color_library_info_alter(), but I can't seem to find what that is.

mermentau’s picture

mermentau CreditAttribution: mermentau commented
Version: 8.0.0-beta4 » 8.0.x-dev

Changed the version as I tested with the core dev (1/25/2015) and the same result.

mermentau’s picture

mermentau CreditAttribution: mermentau commented
Category: Support request » Bug report
FileSize
undefined_index_css_notice-2404253-4.patch2.99 KB

It looks like color.module is looking through all Mayo's libraries and the ones with no css are giving this "Notice". All the Bartik libraries have css so it doesn't produce the "Notice". A small if(isset($libraries[$name]['css'])) { addition seems to fix the problem. Attached is my first core patch attempt.

mermentau’s picture

mermentau CreditAttribution: mermentau commented
Status: Active » Needs review
Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented
Status: Needs review » Reviewed & tested by the community

This should be good.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented

Patch looks good.

+++ b/core/modules/color/color.module
@@ -77,25 +77,27 @@ function color_library_info_alter(&$libraries, $extension) {
+        if(isset($libraries[$name]['css'])) {

Missing space after "if".

Can be fixed upon commit.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Color module is not blessed with many tests which is why we inadvertently break it :(

Status: Needs work » Needs review

wuinfo queued 4: undefined_index_css_notice-2404253-4.patch for re-testing.

wuinfo - Bill Wu’s picture

wuinfo - Bill Wu CreditAttribution: wuinfo - Bill Wu commented
FileSize
undefined_index_css_notice-2404253-10-test.patch1.14 KB
undefined_index_css_notice-2404253-10-whole.patch4.14 KB

I have added a test for this bug. The additional test code itself is quite simple. I just added a dummy library to the color_test_theme YML file to mimic the real-life situation.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

RTBC - if tests fail and pass :).

The last submitted patch, 10: undefined_index_css_notice-2404253-10-test.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed 6c2eb9c and pushed to 8.0.x. Thanks!

The javascript added by this patch does not confirm to our javascript standards as determined by eslint.

diff --git a/core/modules/color/tests/modules/color_test/themes/color_test_theme/js/color_test_theme-fontsize.js b/core/modules/color/tests/modules/color_test/themes/color_test_theme/js/color_test_theme-fontsize.js
index a7c46b2..109f743 100644
--- a/core/modules/color/tests/modules/color_test/themes/color_test_theme/js/color_test_theme-fontsize.js
+++ b/core/modules/color/tests/modules/color_test/themes/color_test_theme/js/color_test_theme-fontsize.js
@@ -3,6 +3,7 @@
  * Adds javascript functions for font resizing.
  */
 (function ($) {
-$(document).ready(function() {});
-})(jQuery);
+  "use strict";
 
+  $(document).ready(function () {});
+})(jQuery);

Fix coding standards.

  • alexpott committed 6c2eb9c on 8.0.x 
    Issue #2404253 by wuinfo, mermentau: Undefined index: css in...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.