Problem/Motivation

See https://www.drupal.org/node/2462717, which is the CR for #1279226: jQuery and Drupal JavaScript libraries and settings are output even when no JS is added to the page.

That recent D7 API addition makes it possible for those Drupal modules whose JS does not rely on jQuery, to not load jQuery.

Google Analytics — when used in its simplest form — is a great example of that.

Proposed resolution

Make a one-line modification to the Google Analytics module so that it informs Drupal 7 that it doesn't depend on jQuery.

Remaining tasks

None.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#8 Issue-2509446-by-wim-hass-Specify-requiresjquery--FA.patch1.55 KBhass
#2 ga_requires_jquery_false-2509446-2.patch523 byteswim leers

Comments

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [1279226 » Specify 'requires_jquery' => FALSE, to not load jQuery if Google Analytics is used in its basic mode
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new ga_requires_jquery_false-2509446-2.patch523 bytes
hass’s picture

hass commented
Status: Needs review » Postponed (maintainer needs more info)

I'm confused what these basic mode should be.

The module code requires jquery... See the js file in the module. There are several event handlers attached and some jquery features used.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Postponed (maintainer needs more info) » Active

Only the module's JS files require jQuery. And the way those are added:

drupal_add_js(drupal_get_path('module', 'googleanalytics') . '/googleanalytics.js');

that still causes jQuery to be attached.

The majority of things just call the ga() function, which doesn't require jQuery.

i.e. this patch just says that the built tracker code does not require jQuery.

hass’s picture

hass commented
Version: 7.x-2.1 » 7.x-2.x-dev
Category: Task » Feature request
Priority: Major » Minor
Status: Active » Needs review
manuel garcia’s picture

manuel garcia commented
Priority: Minor » Normal

Well, this makes a lot of sense, and the patch looks simple enough. Not RTBC'ing because i haven't manually tested the patch, but it looks good to me!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#5: d'oh, all of those metadata changes make sense — apologies, and thank you!

hass’s picture

hass commented
StatusFileSize
new Issue-2509446-by-wim-hass-Specify-requiresjquery--FA.patch1.55 KB

The patch looks incomplete. New patch attached.

The deal to get this in is - fix #2391025: Add support for inline JS/CSS with #attached and I commit this patch here.

  • hass committed d93f2bc on 7.x-2.x 
    Issue #2509446 by wim: Specify 'requires_jquery' => FALSE, to not load...
hass’s picture

hass commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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