Skip to content

Commit

Permalink
Fix outboundLinkTracker bugs in Safari
Browse files Browse the repository at this point in the history
  • Loading branch information
@philipwalton
philipwalton committed Jun 7, 2017
1 parent 7b354a5 commit 7e29fb9
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 46 deletions.
30 changes: 19 additions & 11 deletions lib/plugins/outbound-link-tracker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,31 +85,39 @@ class OutboundLinkTracker {
eventLabel: url.href,
};

/** @type {FieldsObj} */
const userFields = assign({}, this.opts.fieldsObj,
getAttributeFields(link, this.opts.attributePrefix));

const fieldsObj = createFieldsObj(defaultFields, userFields,
this.tracker, this.opts.hitFilter, link, event);

if (!navigator.sendBeacon &&
linkClickWillUnloadCurrentPage(event, link)) {
// Adds a new event handler at the last minute to minimize the chances
// that another event handler for this click will run after this logic.
window.addEventListener('click', function(event) {
const clickHandler = () => {
window.removeEventListener('click', clickHandler);

// Checks to make sure another event handler hasn't already prevented
// the default action. If it has the custom redirect isn't needed.
if (!event.defaultPrevented) {
// Stops the click and waits until the hit is complete (with
// timeout) for browsers that don't support beacon.
event.preventDefault();
defaultFields.hitCallback = withTimeout(function() {

const oldHitCallback = fieldsObj.hitCallback;
fieldsObj.hitCallback = withTimeout(function() {
if (typeof oldHitCallback == 'function') oldHitCallback();
location.href = href;
});
}
});
this.tracker.send('event', fieldsObj);
};
window.addEventListener('click', clickHandler);
} else {
this.tracker.send('event', fieldsObj);
}

/** @type {FieldsObj} */
const userFields = assign({}, this.opts.fieldsObj,
getAttributeFields(link, this.opts.attributePrefix));

this.tracker.send('event',
createFieldsObj(defaultFields, userFields,
this.tracker, this.opts.hitFilter, link, event));
}
}

Expand Down
15 changes: 9 additions & 6 deletions test/e2e/fixtures/outbound-form-tracker.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,32 @@
</head>
<body>

<form action="https://example.com/outbound-submit">
<form method="post" action="https://example.com/?q=outbound-submit">
<input id="outbound-submit" type="submit">
</form>

<form
method="post"
class="form"
action="https://example.com/outbound-submit-with-class">
action="https://example.com/?q=outbound-submit-with-class">
<input id="outbound-submit-with-class" type="submit">
</form>

<form action="/test/e2e/fixtures/blank.html">
<form method="get" action="/test/e2e/fixtures/blank.html">
<input id="local-submit" type="submit">
</form>

<form
action="https://example.com/declarative-attributes-submit"
method="post"
action="https://example.com/?q=declarative-attributes-submit"
ga-event-category="External Form"
ga-dimension-1="true">
<input id="declarative-attributes-submit" type="submit">
</form>

<form
action="https://example.com/declarative-attributes-prefix-submit"
method="post"
action="https://example.com/?q=declarative-attributes-prefix-submit"
data-ga-event-label="www.google-analytics.com"
data-ga-non-interaction="true">
<input id="declarative-attributes-prefix-submit" type="submit">
Expand All @@ -44,7 +47,7 @@
if (shadowHost.attachShadow) {
shadowHost.attachShadow({mode: 'open'});
shadowHost.shadowRoot.innerHTML =
'<form action="https://example.com/shadow-host">' +
'<form method="post" action="https://example.com/?q=shadow-host">' +
' <button>Submit</button>' +
'</form>';
}
Expand Down
14 changes: 7 additions & 7 deletions test/e2e/fixtures/outbound-link-tracker.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@

<a
id="outbound-link"
href="https://example.com/outbound-link">
href="https://example.com/?q=outbound-link">
Outbound link
</a>

<a
id="outbound-link-with-class"
class="link"
href="https://example.com/outbound-link-with-class">
href="https://example.com/?q=outbound-link-with-class">
Outbound link with Class
</a>

Expand All @@ -38,15 +38,15 @@

<a
id="declarative-attributes"
href="https://example.com/declarative-attributes"
href="https://example.com/?q=declarative-attributes"
ga-event-category="External Link"
ga-dimension-1="true">
Declarative Attributes
</a>

<a
id="declarative-attributes-prefix"
href="https://example.com/declarative-attributes-prefix"
href="https://example.com/?q=declarative-attributes-prefix"
data-ga-event-label="example.com"
data-ga-non-interaction="true">
Declarative Attributes
Expand All @@ -61,7 +61,7 @@
viewBox="0 0 100 20"
width="100"
height="20">
<a id="svg-link" xlink:href= "https://example.com/svg-link">
<a id="svg-link" xlink:href= "https://example.com/?q=svg-link">
<text x="0" y="16" font-family="serif" font-size="16">
SVG Link
</text>
Expand All @@ -77,7 +77,7 @@
id="area-link"
shape="rect"
coords="0,0,20,20"
href="https://example.com/area-link">
href="https://example.com/?q=area-link">
</map>
</div>

Expand All @@ -86,7 +86,7 @@
if (shadowHost.attachShadow) {
shadowHost.attachShadow({mode: 'open'});
shadowHost.shadowRoot.innerHTML =
'<a href="https://example.com/shadow-host">Shadow Link</a>';
'<a href="https://example.com/?q=shadow-host">Shadow Link</a>';
}
</script>

Expand Down
2 changes: 1 addition & 1 deletion test/e2e/ga.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function logHitData(testId) {

oldSendHitTask(model);

if ('sendBeacon' in navigator) {
if (typeof navigator.sendBeacon == 'function') {
navigator.sendBeacon(`/collect/${testId}`, hitPayload);
} else {
const beacon = new Image();
Expand Down
12 changes: 6 additions & 6 deletions test/e2e/outbound-form-tracker-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('outboundFormTracker', function() {
const hits = log.getHits();
assert.strictEqual(hits[0].ec, 'Outbound Form');
assert.strictEqual(hits[0].ea, 'submit');
assert.strictEqual(hits[0].el, 'https://example.com/outbound-submit');
assert.strictEqual(hits[0].el, 'https://example.com/?q=outbound-submit');
});

it('does not send events on local form submits', () => {
Expand All @@ -74,7 +74,7 @@ describe('outboundFormTracker', function() {
it('navigates to the proper outbound location on submit', () => {
browser.execute(ga.run, 'require', 'outboundFormTracker');
browser.click('#outbound-submit');
browser.waitUntil(urlMatches('https://example.com/outbound-submit'));
browser.waitUntil(urlMatches('https://example.com/?q=outbound-submit'));
});

it('navigates to the proper local location on submit', () => {
Expand Down Expand Up @@ -108,7 +108,7 @@ describe('outboundFormTracker', function() {
assert.strictEqual(hits[0].ec, 'Outbound Form');
assert.strictEqual(hits[0].ea, 'submit');
assert.strictEqual(
hits[0].el, 'https://example.com/outbound-submit-with-class');
hits[0].el, 'https://example.com/?q=outbound-submit-with-class');
});

it('supports customizing what is considered an outbound form', () => {
Expand Down Expand Up @@ -136,7 +136,7 @@ describe('outboundFormTracker', function() {
const hits = log.getHits();
assert.strictEqual(hits[0].ec, 'External Form');
assert.strictEqual(hits[0].ea, 'send');
assert.strictEqual(hits[0].el, 'https://example.com/outbound-submit');
assert.strictEqual(hits[0].el, 'https://example.com/?q=outbound-submit');
assert.strictEqual(hits[0].ni, '1');
});

Expand Down Expand Up @@ -186,7 +186,7 @@ describe('outboundFormTracker', function() {
const hits = log.getHits();
assert.strictEqual(hits[0].ec, 'Outbound Form');
assert.strictEqual(hits[0].ea, 'submit');
assert.strictEqual(hits[0].el, 'https://example.com/shadow-host');
assert.strictEqual(hits[0].el, 'https://example.com/?q=shadow-host');
});

it('includes usage params with all hits', () => {
Expand Down Expand Up @@ -261,7 +261,7 @@ function requireOutboundFormTracker_shouldTrackOutboundForm() {
function requireOutboundFormTracker_hitFilter() {
ga('require', 'outboundFormTracker', {
hitFilter: (model, form, event) => {
if (form.action == 'https://example.com/outbound-submit') {
if (form.action == 'https://example.com/?q=outbound-submit') {
model.set('eventLabel', '/outbound-submit', true);
}
model.set('dimension1', event.type);
Expand Down
20 changes: 10 additions & 10 deletions test/e2e/outbound-link-tracker-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('outboundLinkTracker', function() {
const hits = log.getHits();
assert.strictEqual(hits[0].ec, 'Outbound Link');
assert.strictEqual(hits[0].ea, 'click');
assert.strictEqual(hits[0].el, 'https://example.com/outbound-link');
assert.strictEqual(hits[0].el, 'https://example.com/?q=outbound-link');
});

it('does not send events on local link clicks', () => {
Expand All @@ -82,7 +82,7 @@ describe('outboundLinkTracker', function() {
it('navigates to the proper location on outbound clicks', () => {
browser.execute(ga.run, 'require', 'outboundLinkTracker');
browser.click('#outbound-link');
browser.waitUntil(urlMatches('https://example.com/outbound-link'));
browser.waitUntil(urlMatches('https://example.com/?q=outbound-link'));
});

it('navigates to the proper location on local clicks', () => {
Expand All @@ -99,7 +99,7 @@ describe('outboundLinkTracker', function() {
const hits = log.getHits();
assert.strictEqual(hits[0].ec, 'Outbound Link');
assert.strictEqual(hits[0].ea, 'click');
assert.strictEqual(hits[0].el, 'https://example.com/svg-link');
assert.strictEqual(hits[0].el, 'https://example.com/?q=svg-link');
});

it('works with <area> links', function() {
Expand All @@ -112,7 +112,7 @@ describe('outboundLinkTracker', function() {
const hits = log.getHits();
assert.strictEqual(hits[0].ec, 'Outbound Link');
assert.strictEqual(hits[0].ea, 'click');
assert.strictEqual(hits[0].el, 'https://example.com/area-link');
assert.strictEqual(hits[0].el, 'https://example.com/?q=area-link');
});

it('supports events other than click', () => {
Expand All @@ -133,11 +133,11 @@ describe('outboundLinkTracker', function() {
const hits = log.getHits();
assert.strictEqual(hits[0].ec, 'Outbound Link');
assert.strictEqual(hits[0].ea, 'mousedown');
assert.strictEqual(hits[0].el, 'https://example.com/outbound-link');
assert.strictEqual(hits[0].el, 'https://example.com/?q=outbound-link');
if (browserSupportsRightClick()) {
assert.strictEqual(hits[1].ec, 'Outbound Link');
assert.strictEqual(hits[1].ea, 'contextmenu');
assert.strictEqual(hits[1].el, 'https://example.com/outbound-link');
assert.strictEqual(hits[1].el, 'https://example.com/?q=outbound-link');
}
});

Expand All @@ -164,7 +164,7 @@ describe('outboundLinkTracker', function() {
assert.strictEqual(hits[0].ec, 'Outbound Link');
assert.strictEqual(hits[0].ea, 'click');
assert.strictEqual(hits[0].el,
'https://example.com/outbound-link-with-class');
'https://example.com/?q=outbound-link-with-class');
});

it('supports customizing what is considered an outbound link', () => {
Expand Down Expand Up @@ -192,7 +192,7 @@ describe('outboundLinkTracker', function() {
const hits = log.getHits();
assert.strictEqual(hits[0].ec, 'External Link');
assert.strictEqual(hits[0].ea, 'tap');
assert.strictEqual(hits[0].el, 'https://example.com/outbound-link');
assert.strictEqual(hits[0].el, 'https://example.com/?q=outbound-link');
assert.strictEqual(hits[0].ni, '1');
});

Expand Down Expand Up @@ -242,7 +242,7 @@ describe('outboundLinkTracker', function() {
const hits = log.getHits();
assert.strictEqual(hits[0].ec, 'Outbound Link');
assert.strictEqual(hits[0].ea, 'click');
assert.strictEqual(hits[0].el, 'https://example.com/shadow-host');
assert.strictEqual(hits[0].el, 'https://example.com/?q=shadow-host');
});

it('includes usage params with all hits', () => {
Expand Down Expand Up @@ -343,7 +343,7 @@ function requireOutboundLinkTracker_shouldTrackOutboundLink() {
function requireOutboundLinkTracker_hitFilter() {
ga('require', 'outboundLinkTracker', {
hitFilter: (model, link, event) => {
if (link.href == 'https://example.com/outbound-link') {
if (link.href == 'https://example.com/?q=outbound-link') {
model.set('eventLabel', '/outbound-link', true);
}
model.set('dimension1', event.type, true);
Expand Down
10 changes: 5 additions & 5 deletions test/e2e/wdio.conf.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ const getCapabilities = () => {
// platform: 'OS X 10.11',
// version: 'latest',
// },
// {
// browserName: 'safari',
// platform: 'OS X 10.12',
// version: '10.0',
// },
{
browserName: 'safari',
platform: 'OS X 10.12',
version: '10.0',
},
// {
// browserName: 'safari',
// platform: 'OS X 10.11',
Expand Down

0 comments on commit 7e29fb9

Please sign in to comment.