Skip to content

feat(server-side): serve GA via first-party server - #17323

Merged
stephendherrera merged 4 commits into
mainfrom
feat/ss-gtm-first-party
Aug 4, 2026
Merged

feat(server-side): serve GA via first-party server#17323
stephendherrera merged 4 commits into
mainfrom
feat/ss-gtm-first-party

Conversation

@stephendherrera

@stephendherrera stephendherrera commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

One-line summary

Add optional GTM_SERVER_URL and GTM_SERVER_PATH settings that load gtm.js from our own sGTM tagging server. Inert until the env vars are set per environment.

Significant changes and points to review

  • bedrock/settings/base.py — two new settings, both empty-or-default so nothing changes until they're set:
    • GTM_SERVER_URL via _normalize_gtm_server_url(), which strips a trailing slash and prepends https:// to a scheme-less value.
    • GTM_SERVER_PATH via _normalize_gtm_server_path(), which is deliberately not normalized beyond adding a missing leading slash, because of the trailing-slash asymmetry above. Defaults to /gtm.js.
  • bedrock/settings/__init__.py — when GTM_SERVER_URL is set, the origin is added to script-src, connect-src, img-src and frame-src. All four are required.
  • Two templates — both attributes are added to both base-protocol.html and firefox/whatsnew/base-new-theme.html, since the latter declares its own <html> tag rather than extending the base. Worth confirming there's no declaration I missed.
  • gtm-snippet.es6.js — the <html> lookup is hoisted to a local since it's now needed three times. GTM_BASE_URL falls back to https://www.googletagmanager.com, and GTM_SCRIPT_PATH is only consulted when a server URL is present, falling back to /gtm.js, because Google's CDN only ever serves from that path. Consent gating (GPC, DNT, EU opt-in, the /thanks/ case) is untouched.

Issue / Bugzilla link

https://mozilla-hub.atlassian.net/browse/WT-1536

Testing

Confirmed no-op while unset. Rendered CONTENT_SECURITY_POLICY["DIRECTIVES"] inside the bedrock_test container and diffed:

  • main with the vars unset vs this branch with them unset identical, zero CSP change. Which also means no existing test can regress.
  • this branch with GTM_SERVER_URL=https://gtm-dev.allizom.org/ → exactly four additions, one origin each into connect-src, frame-src, img-src, script-src, and nothing else. Trailing slash stripped as intended.
  • a bare gtm-dev.allizom.org and the full https://gtm-dev.allizom.org/ produce identical results, confirming the scheme coercion.

With the vars unset, no data-gtm-server-url or data-gtm-server-path attributes render, getAttribute returns null, and the snippet loads from https://www.googletagmanager.com/gtm.js exactly as before.

Unit tests: bedrock/base/tests/test_settings.py covers both normalizers, including cases pinning the trailing slash so a future refactor can't strip it. 17 passing in that file. ruff check, ruff format --check and prettier --check are clean on all changed files.

Confirmed against the dev tagging server: the custom path serves the real container (200, ~389 KB, correct container ID inside), and /gtm.js keeps serving alongside it, so the GTM-side and bedrock-side changes can land in either order with no breakage window.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an optional server-side GTM configuration (GTM_SERVER_URL) so Bedrock can load gtm.js from a first-party sGTM tagging server when enabled, while remaining a no-op when unset. (Review performed using the repository’s custom instructions/AGENTS.md guidance.)

Changes:

  • Add GTM_SERVER_URL setting and plumb it into CSP directives (script-src, connect-src, img-src, frame-src) when configured.
  • Expose the configured tagging-server origin to the front-end via a data-gtm-server-url attribute on <html> (in both relevant base templates).
  • Update the GTM loader snippet to use the configured base URL, falling back to https://www.googletagmanager.com.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
media/js/base/gtm/gtm-snippet.es6.js Load gtm.js from a configurable base URL read from <html data-gtm-server-url>, with Google fallback.
bedrock/settings/base.py Introduces GTM_SERVER_URL env var with basic normalization.
bedrock/settings/init.py Adds GTM_SERVER_URL to CSP directive allowlists when set.
bedrock/firefox/templates/firefox/whatsnew/base-new-theme.html Adds data-gtm-server-url to the page’s <html> tag when configured.
bedrock/base/templates/base-protocol.html Adds data-gtm-server-url to the main base template’s <html> tag when configured.

Comment thread bedrock/settings/base.py Outdated
Comment thread media/js/base/gtm/gtm-snippet.es6.js Outdated
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.93%. Comparing base (b843b13) to head (ccba721).

Files with missing lines Patch % Lines
bedrock/settings/__init__.py 20.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #17323      +/-   ##
==========================================
- Coverage   82.95%   82.93%   -0.02%     
==========================================
  Files         179      179              
  Lines        9362     9377      +15     
==========================================
+ Hits         7766     7777      +11     
- Misses       1596     1600       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread bedrock/settings/base.py Outdated

@stevejalim stevejalim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+wc

Follow-up to review on #17323. The previous guard only added a scheme
when none was present, so http:// and protocol-relative values passed
through untouched. http:// is mixed content on our https pages, and a
protocol-relative value is not a valid CSP source expression, so the
directive entry would be discarded and the script blocked. Normalize to
an https:// origin unconditionally instead, and treat a scheme with no
host as unset rather than letting it through as a truthy nonsense value.
@stephendherrera
stephendherrera merged commit 0736eb1 into main Aug 4, 2026
6 checks passed
@stephendherrera
stephendherrera deleted the feat/ss-gtm-first-party branch August 4, 2026 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants