In Drupal 7 it was possible to attach javascript to forms by using the #attached form API property. In D8 this has changes to using libraries. We have one case where this is used: to attach javascript to dynamically update the help text of vertical tabs.

In this issue let's simply convert the way the javascript is attached so that we do not get fatal errors on the node edit screen, we will later see to the javascript itself.

In #2418547: Replace drupal_add_css() with form API #attached property a similar conversion was done, but for attached CSS.

Change record: Themes now use libraries exclusively, not individual stylesheets or JavaScript files.

Original report by kalinchernev:

I tried to enable Scheduler on the default Article content type.

I get the following error:
Uncaught PHP Exception LogicException: "You are not allowed to use js in #attached" at /var/www/drupal8.dev/public_html/core/includes/common.inc line 969, referer: http://drupal8.dev/admin/structure/types

The node edit form is not reachable after that error.

To reproduce the issue:
1) Install clean Drupal with latest version of Scheduler
2) Apply this patch: https://www.drupal.org/node/2425119
3) Go to http://mysite.dev/admin/structure/types/manage/article and try to enable Scheduler for this content type

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pfrenssen’s picture

So core is throwing exceptions specifically to inform us that we are using obsolete D7 development practices? That's pretty helpful :)

I just had a look and this javascript is related to the vertical tabs. I'm not sure if this javascript still applies in D8, but let's fix the immediate problem by converting this #attached property to a library as per [#2379475]. I will update the issue summary accordingly.

pfrenssen’s picture

Title: Cannot enable scheduler on a content type » Convert #attached javascript to a library
Category: Bug report » Task
Issue summary: View changes
Priority: Critical » Normal
pfrenssen’s picture

Issue tags: +Novice
kalinchernev’s picture

Assigned: Unassigned » kalinchernev
kalinchernev’s picture

pfrenssen, it happens that this topic of library organization is quite new and I didn't know. Thanks for noting this.

The patch introduces a change in addition to #2418547: Replace drupal_add_css() with form API #attached property. Reading https://www.drupal.org/node/2274843 it seems that now both css and js files have to be declared in the libraries.yml file.

It's interesting that the CSS files can be included in a different way depending on their purpose. I used "theme" way to keep only representational approach: https://www.drupal.org/node/2274843#comment-9465125

On another note, it seems to me that the $use_vertical_tabs used at scheduler.edit.inc is not needed any more. D8 contains the vertical tabs by default? It's another topic anyway, in this patch only the check is removed.

kalinchernev’s picture

Assigned: kalinchernev » Unassigned
Status: Active » Needs review
pfrenssen’s picture

Status: Needs review » Reviewed & tested by the community

On another note, it seems to me that the $use_vertical_tabs used at scheduler.edit.inc is not needed any more. D8 contains the vertical tabs by default?

Scheduler has an option to allow the user to decide where to show the per-node settings: either in the vertical tabs or inline in the main form.That is what this is used for. We will indeed re-evaluate this in the future. Drupal 8 allows the administrator to "design" the forms in the interface. The core functionality is quite basic (basically reordering the form elements) but there will be contributed modules that will extend this, so we might not really need this any more.

+++ b/scheduler.libraries.yml
@@ -0,0 +1,9 @@
+scheduler/admin:
+  css:
+    theme:
+      css/scheduler.css: {}

Oops, apparently this was forgotten in #2418547: Replace drupal_add_css() with form API #attached property, thanks for catching this!

pfrenssen’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x-1.x, awesome work, thanks!

Status: Fixed » Closed (fixed)

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