Skip to content

Commit b459249

Browse files
committed
Unify browser dimension checks
Should address issues where NaN values are included in generated values for document/viewport/screen size dimensions, which cause events to be invalid when they fail to match the regex pattern that validates these fields. Issue: AISP-527
1 parent 24e8245 commit b459249

File tree

3 files changed

+37
-26
lines changed

3 files changed

+37
-26
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@snowplow/browser-tracker-core",
5+
"comment": "Unify browser dimension checks to avoid including NaN values",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@snowplow/browser-tracker-core"
10+
}

libraries/browser-tracker-core/src/helpers/browser_props.ts

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,6 @@ function initializeResizeObserver() {
4646

4747
let cachedProperties: BrowserProperties;
4848

49-
/* Separator used for dimension values e.g. widthxheight */
50-
const DIMENSION_SEPARATOR = 'x';
51-
5249
/**
5350
* Gets various browser properties (that are expensive to read!)
5451
* - Will use a "ResizeObserver" approach in modern browsers to update cached properties only on change
@@ -75,9 +72,9 @@ export function getBrowserProperties() {
7572
*/
7673
function readBrowserProperties(): BrowserProperties {
7774
return {
78-
viewport: floorDimensionFields(detectViewport()),
79-
documentSize: floorDimensionFields(detectDocumentSize()),
80-
resolution: floorDimensionFields(detectScreenResolution()),
75+
viewport: detectViewport(),
76+
documentSize: detectDocumentSize(),
77+
resolution: detectScreenResolution(),
8178
colorDepth: screen.colorDepth,
8279
devicePixelRatio: window.devicePixelRatio,
8380
cookiesEnabled: window.navigator.cookieEnabled,
@@ -109,7 +106,7 @@ function detectViewport() {
109106
height = e['clientHeight'];
110107
}
111108

112-
return Math.max(0, width) + DIMENSION_SEPARATOR + Math.max(0, height);
109+
return makeDimension(Math.max(0, width), Math.max(0, height));
113110
}
114111

115112
/**
@@ -124,21 +121,21 @@ function detectDocumentSize() {
124121
be = document.body,
125122
// document.body may not have rendered, so check whether be.offsetHeight is null
126123
bodyHeight = be ? Math.max(be.offsetHeight, be.scrollHeight) : 0;
127-
var w = Math.max(de.clientWidth, de.offsetWidth, de.scrollWidth);
128-
var h = Math.max(de.clientHeight, de.offsetHeight, de.scrollHeight, bodyHeight);
129-
return isNaN(w) || isNaN(h) ? '' : w + DIMENSION_SEPARATOR + h;
124+
125+
return makeDimension(
126+
Math.max(de.clientWidth, de.offsetWidth, de.scrollWidth),
127+
Math.max(de.clientHeight, de.offsetHeight, de.scrollHeight, bodyHeight)
128+
);
130129
}
131130

132131
function detectScreenResolution() {
133-
return screen.width + DIMENSION_SEPARATOR + screen.height;
132+
return makeDimension(screen.width, screen.height);
134133
}
135134

136-
export function floorDimensionFields(field?: string | null) {
137-
return (
138-
field &&
139-
field
140-
.split(DIMENSION_SEPARATOR)
141-
.map((dimension) => Math.floor(Number(dimension)))
142-
.join(DIMENSION_SEPARATOR)
143-
);
135+
export function makeDimension(width: number, height: number): string | null {
136+
if (isNaN(width) || isNaN(height)) {
137+
return null;
138+
}
139+
140+
return Math.floor(width) + 'x' + Math.floor(height);
144141
}

libraries/browser-tracker-core/test/browser_props.test.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
1-
import { floorDimensionFields, getBrowserProperties } from '../src/helpers/browser_props';
1+
import { makeDimension, getBrowserProperties } from '../src/helpers/browser_props';
22

33
describe('Browser props', () => {
4-
it('floorDimensionFields correctly floors dimension type values', () => {
4+
it('makeDimension correctly floors dimension type values', () => {
55
const testDimensions = '100x100';
6-
expect(floorDimensionFields(testDimensions)).toEqual(testDimensions);
6+
expect(makeDimension(100, 100)).toEqual(testDimensions);
77
});
88

9-
it('floorDimensionFields correctly floors dimension type values with fractional numbers', () => {
10-
const testFractionalDimensions = '100.2x100.1';
11-
expect(floorDimensionFields(testFractionalDimensions)).toEqual('100x100');
9+
it('makeDimension correctly floors dimension type values with fractional numbers', () => {
10+
expect(makeDimension(100.2, 100.1)).toEqual('100x100');
11+
});
12+
13+
it('makeDimension correctly drops invalid values', () => {
14+
expect(makeDimension(undefined as any, 100.1)).toEqual(null);
15+
expect(makeDimension(NaN, 1)).toEqual(null);
1216
});
1317

1418
describe('#getBrowserProperties', () => {
@@ -17,7 +21,7 @@ describe('Browser props', () => {
1721
// @ts-expect-error
1822
document = undefined;
1923
});
20-
24+
2125
it('does not invoke the resize observer if the document is null', () => {
2226
const browserProperties = getBrowserProperties();
2327
expect(browserProperties).not.toEqual(null);

0 commit comments

Comments
 (0)