Skip to content
This repository has been archived by the owner on Mar 25, 2022. It is now read-only.

CTM core experience running when on enhanced experience #83

Closed
markstephens opened this issue Nov 5, 2015 · 13 comments
Closed

CTM core experience running when on enhanced experience #83

markstephens opened this issue Nov 5, 2015 · 13 comments
Labels
bug Something isn't working

Comments

@markstephens
Copy link
Contributor

We have an issue with the core experience - in certain browsers - its loading the fallback image when the page is in enhanced mode.

The core experience looks like this:

<div class="o-tracking o--if-no-js" data-o-component="o-tracking">
    <div style="background:url('...');"></div>
</div>

I've seen this image loading in enhanced mode using ie9, but membership have reported it in IE8 too.

Apparently this quirk won't have ever have worked for IE: http://justinmarsan.com/hidden-elements-and-http-requests/

IE, like WebKit will download background-images even if they have display: none;

That chap's test page confirms it's failing in my IE9 too.

Anyone got any ideas for a solution?

cc @AlbertoElias @triblondon

@markstephens markstephens added the bug Something isn't working label Nov 5, 2015
@markstephens
Copy link
Contributor Author

I've been testing IE's against o-tracking.

  • 8 is ok as uses fallback only
  • 9 is bad and the duplicate requests go through.
  • 10 seems ok
  • 11 seems ok

So it seems to be only IE9.

@triblondon
Copy link
Contributor

@alicebartlett Do you know what GDS do for this kind of thing?

@alicebartlett
Copy link
Member

I studiously avoided all the tracking js for GOV.UK, but I had a look at the code (https://github.com/alphagov/govuk_frontend_toolkit/tree/master/javascripts/govuk/analytics) and asked @edds.

We didn't have a non-javascript solution for tracking and we used google analytics + some custom wrapping for the JS based tracking

@alicebartlett
Copy link
Member

@markstephens @triblondon

So i've thrown together a simple test case and tested it on browerstack and ie9 doesn't download the image for the following code:

<div  style='display:none;' class="o-tracking o--if-no-js" data-o-component="o-tracking">
  <div id="test" style="background:url('ft.png');height:300px;width:300px">
  </div>
</div>

As long as the CSS to set

.core .o--if-js,
.enhanced .o--if-no-js { display: none !important; }

is being loaded in the head, this should work so I'm pretty puzzled now.
Could we get together next week and have a look at it?

@markstephens
Copy link
Contributor Author

@alicebartlett - thanks for looking into it - sorry for late reply, I've been out of office.

It would be great to look at it together, maybe tomorrow (Thursday)? We think it's happening in IE11 now too, as well as IE9.

@commuterjoy
Copy link
Member

Perhaps drop support for non-javascript tracking and implement a basic non-ctm version of o-tracking which would work on IE8 etc. I think this is simpler.

@wheresrhys
Copy link
Contributor

Fixed in next by using the following instead of the recommended non-ctm fallback

    <script>
        (function () {
            if (!cutsTheMustard) {
                var img = new Image();
                img.src = 'https://spoor-api.ft.com/px.gif?data=%7b%22category%22%3a%22page%22%2c%22action%22%3a%22view%22%2c%22system%22%3a%7b%22source%22%3a%22non-ctm%22%7d%2c%22user%22%3a%7b%7d%2c%22context%22%3a%7b%22product%22%3a%22next%22%7d%7d';
            }
        })();
    </script>
    <noscript data-o-component="o-tracking">
        <img src="https://spoor-api.ft.com/px.gif?data=%7b%22category%22%3a%22page%22%2c%22action%22%3a%22view%22%2c%22system%22%3a%7b%22source%22%3a%22non-ctm%22%7d%2c%22user%22%3a%7b%7d%2c%22context%22%3a%7b%22product%22%3a%22next%22%7d%7d"/>
    </noscript>

Mark has implemented something similar on FT.com

It's annoying having to specify the url twice, although it might be possible to read textContent of the noscript to get at it in the non-ctm code.

Do we want to update the docs to recommend something like this instead of the bg image @markstephens @alicebartlett

@markstephens
Copy link
Contributor Author

Do we want to update the docs to recommend something like this instead of the bg image

I think yes, as the CSS version is junk.

might be possible to read textContent of the noscript to get at it in the non-ctm code

We've investigated reading the contents of noscript before, from memory, I think it wasn't reliable. Dan Searle (now left FT) looked into it a lot.

@wheresrhys
Copy link
Contributor

It'd be really nice if ctm, became a standard, so you could properly disable javascript for a page if you wanted to. Something like window.disableScript(), which'd only be available to inline scripts present in the page source. Then you could just noscript it

@triblondon
Copy link
Contributor

See https://discourse.wicg.io/t/disable-scripts-api-standardised-cut-the-mustard-failure-handling/1277 where Rhys raised this on WICG. I came up with the idea of using CSP, and it occured to me that this would already work today, in browsers that support CSP (so this is a bit academic as there probably aren't any that support CSP but don't support enough JS to be considered primary experience)

Anyway, you could do this:

if (!cutsTheMustard) {
  document.cookie = 'CTM=core';
  window.reload();
}

And then in Fastly, some VCL that matches on this:

if (req.http.Cookie ~ /CTM=core/) {
  resp.http.Content-Security-Policy = "script-src 'none'";
}

When the page reloaded, it would not run any JS. However, obviously in browsers that fail CTM and don't support CSP, you would get an endless reload loop. And this doesn't help make noscript work on those browsers.

@wheresrhys
Copy link
Contributor

Oooh clever. But won't that
A) cause lots of csp warnings
B) would it have any effect on inline scripts?
C) have to be careful to have a short lifespan/unset the cookie in case the
browser updates
D) still need to handle the case of the first load of the site

On Saturday, 30 January 2016, Andrew Betts notifications@github.com wrote:

See
https://discourse.wicg.io/t/disable-scripts-api-standardised-cut-the-mustard-failure-handling/1277
where Rhys raised this on WICG. I came up with the idea of using CSP, and
it occured to me that this would already work today, in browsers that
support CSP (so this is a bit academic as there probably aren't any that
support CSP but don't support enough JS to be considered primary experience)

Anyway, you could do this:

if (!cutsTheMustard) {
document.cookie = 'CTM=core';
window.reload();
}

And then in Fastly, some VCL that matches on this:

if (req.http.Cookie ~ /CTM=core/) {
resp.http.Content-Security-Policy = "script-src 'none'";
}


Reply to this email directly or view it on GitHub
#83 (comment)
.


This email was sent by a company owned by Financial Times Group Limited
("FT Group http://aboutus.ft.com/corporate-information/#axzz3rajCSIAt"),
registered office at Number One Southwark Bridge, London SE1 9HL.
Registered in England and Wales with company number 879531. This e-mail may
contain confidential information. If you are not the intended recipient,
please notify the sender immediately, delete all copies and do not
distribute it further. It could also contain personal views which are not
necessarily those of the FT Group. We may monitor outgoing or
incoming emails as permitted by law.

@triblondon
Copy link
Contributor

A) cause lots of csp warnings

In the console, yes. Don't really see that as an issue.

B) would it have any effect on inline scripts?

Yes, the presence of a CSP header disables all inline scripts by default.

C) have to be careful to have a short lifespan/unset the cookie in case the browser updates

Fair enough

D) still need to handle the case of the first load of the site

True, but if the CTM test is in the first packet of the first response it wouldn't make a massive perf impact.

Anyway, it's not practical for us because CSP is not supported in old browsers - and I suppose the whole principle that we're talking about here is counter to the principle of progressive enhancement. We should be turning script ON, not turning it OFF. And ultimately the thing that kicked this off was a lack of an effective <noscript> in core experience, which any CSP powered thing wouldn't solve anyway.

@JakeChampion
Copy link
Contributor

This has been fixed in the Origami documentation for how to implement a cuts-the-mustard test and how to load images only in browsers which do not pass the cuts-the-mustard test

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants