1458197 - Inline DNTHelper

Status: RESOLVED FIXED

(developer.mozilla.org Graveyard :: Performance, enhancement)

Description

[Schalk Neethling [:espressive]]

I have been doing some reading on why one would put GA at the top of the page. The general consensus is that it is because Google said so, but there really is no good reason for it.

Worst case you loose tracking of page views where the user navigated away before the page loaded. I agree with the general feeling here that, for the performance implication, losing these "views" is not worth it.

For reference
--------------

https://github.com/h5bp/html5-boilerplate/blob/6.0.1/dist/doc/html.md#google-universal-analytics-tracking-code

https://github.com/h5bp/html5-boilerplate/commit/2dfb90309f71f14d5037b0da079c1be09464addd#commitcomment-16054505

https://mathiasbynens.be/notes/async-analytics-snippet#comment-50

I therefore propose that we move DNTHelper and the GA code to the bottom of the HTML source and mark them as async

This prevents the current blocking behaviour. Thoughts?

Comment 1

[John Whitlock [:jwhitlock]]

I believe the DNT tracker is needed near the top, so that we can decide to exclude a visitor from the Traffic Cop experiment. If we can segment Traffic Cop pages, then we could do something clever, but I hate doing clever things, they tend to break. The DNT tracker is our own static code, which should be fast, and could be potentially inlined.

I've de-obfuscated the GA snippet like they have in that first link, and I'm encouraged that more experienced JS developers have come to the same conclusion. The GA snippet has two parts. The first part sets up a global object to queue analytics events, and this needs to be installed as early as possible. The second part is setting up an include of the GA script that blocks until loaded, and then uploads any queued events to Google. This second part could be moved down to the bottom of the page. The impact would be to lose page hit data for users that leave a page before it completely loads. This would impact slower pages (with live or interactive examples) more than fast pages. It would be hard to estimate the impact without doing it, and it would be hopefully offset by better SEO results from improved page load. Of course, if you implement it incorrectly, you'll lose a lot more hits.

I'd like to wait to make this change, to see the impact of the CDN changes, and to give our performance auditor a chance to weigh in.

Comment 2

[Schalk Neethling [:espressive]]

(In reply to John Whitlock [:jwhitlock] from comment #1)
> I believe the DNT tracker is needed near the top, so that we can decide to
> exclude a visitor from the Traffic Cop experiment. If we can segment Traffic
> Cop pages, then we could do something clever, but I hate doing clever
> things, they tend to break. The DNT tracker is our own static code, which
> should be fast, and could be potentially inlined.
> 
> I've de-obfuscated the GA snippet like they have in that first link, and I'm
> encouraged that more experienced JS developers have come to the same
> conclusion. The GA snippet has two parts. The first part sets up a global
> object to queue analytics events, and this needs to be installed as early as
> possible. The second part is setting up an include of the GA script that
> blocks until loaded, and then uploads any queued events to Google. This
> second part could be moved down to the bottom of the page. The impact would
> be to lose page hit data for users that leave a page before it completely
> loads. This would impact slower pages (with live or interactive examples)
> more than fast pages. It would be hard to estimate the impact without doing
> it, and it would be hopefully offset by better SEO results from improved
> page load. Of course, if you implement it incorrectly, you'll lose a lot
> more hits.
> 
> I'd like to wait to make this change, to see the impact of the CDN changes,
> and to give our performance auditor a chance to weigh in.

I agree that this is something we should point Tim to. Thanks for the feedback John.

Comment 3

[Kadir Topal [:atopal]]

The DNT check is actually render blocking and one of the things that "Pagespeed Insights" complains about, doesn't look like that's the case with the GA code.

Recently came across that when comparing similar pages between w3schools and MDN.

MDN gets first contentful paint in less than 1s to 42% of users, w3schools does that for 82%.

MDN: https://developers.google.com/speed/pagespeed/insights/?url=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FWeb%2FJavaScript%2FReference%2FGlobal_Objects%2FArray%2FforEach&tab=desktop

w3schools: https://developers.google.com/speed/pagespeed/insights/?hl=en-US&utm_source=PSI&utm_medium=incoming-link&utm_campaign=PSI&url=https%3A%2F%2Fwww.w3schools.com%2Fjsref%2Fjsref_forEach.asp&tab=desktop

That said, I agree that it would be good to hear from our auditor about the impact of this.

Comment 4

[Florian Scholz (Open Web Docs)]

Kadir, please get this one to Tim

Comment 5

[Kadir Topal [:atopal]]

We discussed that and decided to inline the DNTHelper, but not move the GA code, so as to not change the way we measure traffic. According to Tim, the DNTHelper impact is significant, whereas the GA code is not.

See: https://github.com/mdn/sprints/issues/257

Comment 6

[[github robot]]

Commit pushed to master at https://github.com/mozilla/kuma

https://github.com/mozilla/kuma/commit/4a3f945e757b173ec85b5c13d81a0d8d82a3d256
bug 1458197: Inline dnt-helper (#4881)

Inline the minimized dnt-helper JS code instead of including as a script.

Comment 7

[Schalk Neethling [:espressive]]

As suggested by John in the latest pull request, we can move {% include "js/libs/mozilla.dnthelper.min.js" %} inside the script tag in includes/google_analytics.html

We then have only one script element, and one include.

Comment 8

[[github robot]]

Commit pushed to master at https://github.com/mozilla/kuma

https://github.com/mozilla/kuma/commit/7296bee281dbfee76fd675f0ca47d60598d2fcc5
bug 1458197: Merge DNT and GA inline scripts (#4915)

Merge the minimized Mozilla DNT Helper and the Google Analytics setup into a single inline ``<head>`` script.

Comment 9

[John Whitlock [:jwhitlock]]

As mentioned in comment 5, the big win is inlining the DNT script, and we want to avoid changing the GA code because of the risk to our key results tracking.