Problem/Motivation
The documentation for this module states that it will "automatically add the required Javascript [tracking code] to every page generated by Drupal." However, content security policies generally do not allow inline scripts to be included. Console errors look something like: "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”)."
Steps to reproduce
Install the Google Analytics module as described. Ensure that your CSP does not include 'unsafe-inline' for the script-src directive.
Proposed resolution
Following the approach taken to patch the Drupal 7 version of this module (#3170031), move the generated script into a separate file.
Remaining tasks
Do it!
User interface changes
Possibly add this feature as an option, rather than as a default implementation change.
API changes
Not anticipating any...
Data model changes
Likely just a single added field to the module configuration.
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | interdiff.txt_17-21.txt | 915 bytes | brandon_ |
| #21 | 3276807-include-google-analytics--21.patch | 31.67 KB | brandon_ |
| #18 | 3276807-include-google-analytics--17.patch | 31.63 KB | r.van.doorn |
| #15 | 3276807-include-google-analytics--15.patch | 35.81 KB | beunerd |
| #13 | 3276807-include-google-analytics--13.patch | 35.8 KB | beunerd |
Issue fork google_analytics-3276807
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
beunerd commentedHere's a patch that attaches a separate JS file by default, but can be toggled to insert inline if preferred.
Comment #5
beunerd commentedThe patch in #3 keeps failing tests (against Drupal 9.3 and 9.2), but none appear to be related/caused by these updates...
or am I missing something? Edit: Yeah, I was missing something.Comment #6
beunerd commentedNow including the async gtag script
Comment #8
beunerd commentedAddressing tests.
Comment #9
beunerd commentedRenaming the patch file so it's consistent with the comment #.
Comment #10
beunerd commentedMarking this as needs review.
Comment #11
nicholassJust noting for others, there is another module which is compatible with the CSP module called.
https://www.drupal.org/project/ga
I think it solves the problem a little differently
Comment #12
beunerd commentedHere's an updated patch.
Comment #13
beunerd commentedChanged the relative path from using `realpath` to the `file_assets_path` to ensure it can be accessed from the resulting markup / HTML.
Comment #15
beunerd commentedMissed a slash at the beginning of the relative path.
Comment #16
ricovandevin commentedChanged status to Needs review to trigger tests.
Comment #18
r.van.doorn commentedThe patch from #15 isn't working for me. The file_assets_path settings that is used for the relative path is empty in my project. I don't like the idea of adding a hardcoded filepath to my own settings file so I've added a fallback to use the public path if the assets scheme doesn't exist.
I also fixed some coding standard issues I ran into while figuring out why the relative path wasn't working.
Comment #19
beunerd commentedTriggering tests
Comment #20
brandon_ commentedThe patch in #18 seems to be working for me as well. Thanks very much.
Hopefully this can get picked up, as it seems to be the best way for GA4 on a site with a strong CSP.
Comment #21
brandon_ commentedAfter updating to Drupal 10.1, the patch in #18 stopped working for me. When the url was being built, it didn't properly parse out the assets folder, so I ended up with pages linking to
src="//js/google-analytics-XX".In my case, I don't have an assets folder specified, but with the logic in the patch, it comes back as existing and uses the same value as the public folder. I found that a small change from
$settings->get('file_assets_path')toAssetsStream::basePath()made the difference for me.Otherwise the rest of the changes from r.van.doorn work great. I'm uploading my patch which is basically all of r.van.doorn's work with two lines changed, as well as an interdiff. This works for me on D10.1, and still seems to work as expected on D9.5 as well.
Comment #22
eelkeblokThere is a merge request, can that be updated based on the latest changes? Patch workflow is not going to stick around for the long haul, best get to grips with the MR workflow. Also, a quick glance at the patch told me that there are some unrelated code changes in the patch. It is best to limit code changes for an issue to the changes required to solve the issue at hand; it limits the chance of merge conflicts and reduces the number of lines to review.
Comment #23
eelkeblokI updated the MR with the latest changes from #21 and merged in 4.x in the process. I'm looking into the implications of using the AssetsStream wrapper directly... I don't think this will work in the current form with Drupal 9.3, which is currently the oldest core version this module claims to support.
Comment #24
eelkeblokJust noticed the deprecated notice on the module status and found more information in the documentation (maybe a bit more information wouldn't hurt on the project page either).
Anyway, I rolled back some unrelated changes, which hopefully makes reviewing this a little easier. However, with tests out due to the move to GitLab, I have little faith in this actually ever making the finish line...
Comment #26
japerryUnfortunately I do not believe such a large change will occur in this module, due to its EOL status. Working to update the documentation so that users will goto the Google Tag module instead. Fortunately the Google Tag module incorporates the goals of this patch, so please try it out!
Comment #27
eelkeblokThanks. The customer was already in the process of moving to GTM, so this was a nice extra nudge :)