Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/pkgsite: Google Tag Manager is loaded twice #40321

Closed
andybons opened this issue Jul 21, 2020 · 8 comments
Closed

x/pkgsite: Google Tag Manager is loaded twice #40321

andybons opened this issue Jul 21, 2020 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. pkgsite
Milestone

Comments

@andybons
Copy link
Member

The following snippet:

f = document.createElement("iframe")
f.src = gtmURL;
f.height = "0";
f.width = "0";
f.style = "display:none;visibility:hidden"
s = document.createElement("noscript");
s.appendChild(f);
document.head.appendChild(s);

Is meant to replace the following:

<noscript><iframe src="https://www.googletagmanager.com/ns.html?id=GTM-W8MVQXG" height="0" width="0" style="display:none;visibility:hidden"></iframe></noscript>

However, the <noscript> tag is explicitly a fallback for when JavaScript is not available, so replacing it with JavaScript doesn’t achieve its desired purpose (and ends up loading the GTM snippet again within the iframe).

As I understand it, this was done for security reasons.

/cc @jba @jamalc

@andybons andybons added NeedsFix The path to resolution is known, but the work has not been done. pkgsite labels Jul 21, 2020
@andybons andybons added this to the Unreleased milestone Jul 21, 2020
@jba jba self-assigned this Jul 21, 2020
@jba
Copy link
Contributor

jba commented Jul 21, 2020

That change was made in golang/pkgsite@1940919. The author didn't appreciate the meaning of noscript.

We still have the problem that we can't use nonces, so I'm not sure how to generate the iframe securely.
@empijei
@FiloSottile

@gopherbot
Copy link

Change https://golang.org/cl/243858 mentions this issue: content/static/html: remove bad JS

gopherbot pushed a commit to golang/pkgsite that referenced this issue Jul 21, 2020
There was an attempt to create a noscript tag, which only runs when JS
is disabled, using JS.

Remove it.

For golang/go#40321.

Change-Id: I99c02810ed7c299fb606259823ef9b764c525bb6
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/243858
Reviewed-by: Julie Qiu <julie@golang.org>
@FiloSottile
Copy link
Contributor

We still have the problem that we can't use nonces, so I'm not sure how to generate the iframe securely.

I am not the expert, but I think you can use nonces, as long as you wire them through the template so they only get applied where you intend them to be applied. I can review a CL that introduces nonces if you'd like.

@jba
Copy link
Contributor

jba commented Jul 27, 2020

My understanding (from @empijei) is that the nonce isn't necessary.

But there is another problem with restoring the noscript iframe: the GTM ID is now in the DOM (rather than part of the template data). Can it be accessed without JS?

@jba
Copy link
Contributor

jba commented Jul 27, 2020

@jamalc

@gopherbot
Copy link

Change https://golang.org/cl/245557 mentions this issue: content/static/html: restore GTM iframe

@andybons
Copy link
Member Author

But there is another problem with restoring the noscript iframe: the GTM ID is now in the DOM (rather than part of the template data). Can it be accessed without JS?

Can we include it in the template data?

@julieqiu julieqiu changed the title pkgsite: Google Tag Manager is loaded twice x/pkgsite: Google Tag Manager is loaded twice Jul 29, 2020
@jba
Copy link
Contributor

jba commented Jul 29, 2020

There was a reason we removed it in the first place...not sure what it was.

@golang golang locked and limited conversation to collaborators Aug 21, 2021
@rsc rsc unassigned jba Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. pkgsite
Projects
None yet
Development

No branches or pull requests

4 participants