Skip to content

Commit fe977c0

Browse files
authored
🐛 amp-analytics failing in some cases when using amp shadow (ampproject#39498)
* fix: amp-pixel error for shadow dom * fix args
1 parent ecf5bcc commit fe977c0

File tree

5 files changed

+73
-15
lines changed

5 files changed

+73
-15
lines changed

Diff for: extensions/amp-analytics/0.1/test/test-transport.js

+3
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ describes.realWin(
4040
sendXhrStub = env.sandbox.stub();
4141
sendBeaconStub = env.sandbox.stub();
4242

43+
env.sandbox.spy(Services, 'urlReplacementsForDoc');
44+
4345
// Needed for PreconnectService.
4446
installDocService(win, true);
4547
});
@@ -578,6 +580,7 @@ describes.realWin(
578580

579581
function expectImagePixel(url, referrerPolicy, attributionSrc) {
580582
imagePixelVerifier.verifyRequest(url, referrerPolicy, attributionSrc);
583+
expect(Services.urlReplacementsForDoc).to.be.calledWith(ampdoc);
581584
}
582585

583586
function expectNoImagePixel() {

Diff for: extensions/amp-analytics/0.1/transport.js

+13-3
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,8 @@ export class Transport {
141141
getRequest(false),
142142
suppressWarnings,
143143
/** @type {string|undefined} */ (this.referrerPolicy_),
144-
/** @type {string|undefined} */ (this.attributionSrc_)
144+
/** @type {string|undefined} */ (this.attributionSrc_),
145+
this.ampdoc_
145146
);
146147
return;
147148
}
@@ -248,18 +249,27 @@ export class Transport {
248249
* @param {boolean} suppressWarnings
249250
* @param {string|undefined} referrerPolicy
250251
* @param {string|undefined} attributionSrc
252+
* @param {!Element|!./service/ampdoc-impl.AmpDoc} elementOrAmpDoc Whether services are provided by an
253+
* element.
251254
*/
252255
static sendRequestUsingImage(
253256
win,
254257
request,
255258
suppressWarnings,
256259
referrerPolicy,
257-
attributionSrc
260+
attributionSrc,
261+
elementOrAmpDoc
258262
) {
259263
if (!win) {
260264
return;
261265
}
262-
const image = createPixel(win, request.url, referrerPolicy, attributionSrc);
266+
const image = createPixel(
267+
win,
268+
request.url,
269+
referrerPolicy,
270+
attributionSrc,
271+
elementOrAmpDoc
272+
);
263273
loadPromise(image)
264274
.then(() => {
265275
dev().fine(TAG_, 'Sent image request', request.url);

Diff for: src/builtins/amp-pixel/amp-pixel.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ export class AmpPixel extends BaseElement {
8585
this.win,
8686
src,
8787
this.referrerPolicy_,
88-
this.element.getAttribute('attributionsrc')
88+
this.element.getAttribute('attributionsrc'),
89+
this.element
8990
);
9091
dev().info(TAG, 'pixel triggered: ', src);
9192
return pixel;

Diff for: src/pixel.js

+51-11
Original file line numberDiff line numberDiff line change
@@ -18,28 +18,44 @@ const TAG = 'pixel';
1818
* @param {string} src
1919
* @param {?string=} referrerPolicy
2020
* @param {string=} attributionSrc
21+
* @param {(Element|./service/ampdoc-impl.AmpDoc)=} opt_elementOrAmpDoc Whether services are provided by an
22+
* element.
2123
* @return {!Element}
2224
*/
23-
export function createPixel(win, src, referrerPolicy, attributionSrc) {
25+
export function createPixel(
26+
win,
27+
src,
28+
referrerPolicy,
29+
attributionSrc,
30+
opt_elementOrAmpDoc
31+
) {
2432
// Caller need to verify window is not destroyed when creating pixel
2533
if (referrerPolicy && referrerPolicy !== 'no-referrer') {
2634
user().error(TAG, 'Unsupported referrerPolicy: %s', referrerPolicy);
2735
}
2836

2937
return referrerPolicy === 'no-referrer'
30-
? createNoReferrerPixel(win, src, attributionSrc)
31-
: createImagePixel(win, src, false, attributionSrc);
38+
? createNoReferrerPixel(win, src, attributionSrc, opt_elementOrAmpDoc)
39+
: createImagePixel(win, src, false, attributionSrc, opt_elementOrAmpDoc);
3240
}
3341

3442
/**
3543
* @param {!Window} win
3644
* @param {string} src
3745
* @param {string=} attributionSrc
46+
* @param {(Element|./service/ampdoc-impl.AmpDoc)=} opt_elementOrAmpDoc Whether services are provided by an
47+
* element.
3848
* @return {!Element}
3949
*/
40-
function createNoReferrerPixel(win, src, attributionSrc) {
50+
function createNoReferrerPixel(win, src, attributionSrc, opt_elementOrAmpDoc) {
4151
if (isReferrerPolicySupported()) {
42-
return createImagePixel(win, src, true, attributionSrc);
52+
return createImagePixel(
53+
win,
54+
src,
55+
true,
56+
attributionSrc,
57+
opt_elementOrAmpDoc
58+
);
4359
} else {
4460
// if "referrerPolicy" is not supported, use iframe wrapper
4561
// to scrub the referrer.
@@ -52,7 +68,13 @@ function createNoReferrerPixel(win, src, attributionSrc) {
5268
}
5369
);
5470
iframe.onload = () => {
55-
createImagePixel(iframe.contentWindow, src);
71+
createImagePixel(
72+
iframe.contentWindow,
73+
src,
74+
undefined,
75+
undefined,
76+
opt_elementOrAmpDoc
77+
);
5678
};
5779
win.document.body.appendChild(iframe);
5880
return iframe;
@@ -64,9 +86,17 @@ function createNoReferrerPixel(win, src, attributionSrc) {
6486
* @param {string} src
6587
* @param {boolean=} noReferrer
6688
* @param {string=} attributionSrc
89+
* @param {(Element|./service/ampdoc-impl.AmpDoc)=} opt_elementOrAmpDoc Whether services are provided by an
90+
* element.
6791
* @return {!Image}
6892
*/
69-
function createImagePixel(win, src, noReferrer = false, attributionSrc) {
93+
function createImagePixel(
94+
win,
95+
src,
96+
noReferrer = false,
97+
attributionSrc,
98+
opt_elementOrAmpDoc
99+
) {
70100
const Image = WindowInterface.getImage(win);
71101
const image = new Image();
72102
if (noReferrer) {
@@ -82,7 +112,8 @@ function createImagePixel(win, src, noReferrer = false, attributionSrc) {
82112
const substituteVariables =
83113
getAttributionReportingStatusUrlVariableRewriter(
84114
win,
85-
attributionReportingStatus
115+
attributionReportingStatus,
116+
opt_elementOrAmpDoc
86117
);
87118
attributionSrc = substituteVariables(attributionSrc);
88119
image.attributionSrc = attributionSrc;
@@ -93,7 +124,8 @@ function createImagePixel(win, src, noReferrer = false, attributionSrc) {
93124
}
94125
const substituteVariables = getAttributionReportingStatusUrlVariableRewriter(
95126
win,
96-
attributionReportingStatus
127+
attributionReportingStatus,
128+
opt_elementOrAmpDoc
97129
);
98130
src = substituteVariables(src);
99131
image.src = src;
@@ -113,13 +145,21 @@ function isReferrerPolicySupported() {
113145
/**
114146
* @param {!Window} win
115147
* @param {string=} status
148+
* @param {(Element|./service/ampdoc-impl.AmpDoc)=} opt_elementOrAmpDoc Whether services are provided by an
149+
* element.
116150
* @return {function(string): string}
117151
*/
118-
function getAttributionReportingStatusUrlVariableRewriter(win, status) {
152+
function getAttributionReportingStatusUrlVariableRewriter(
153+
win,
154+
status,
155+
opt_elementOrAmpDoc
156+
) {
119157
const substitutionFunctions = {
120158
'ATTRIBUTION_REPORTING_STATUS': () => status,
121159
};
122-
const replacements = Services.urlReplacementsForDoc(win.document);
160+
const replacements = Services.urlReplacementsForDoc(
161+
opt_elementOrAmpDoc || win.document
162+
);
123163
const allowlist = {
124164
'ATTRIBUTION_REPORTING_STATUS': true,
125165
};

Diff for: test/unit/builtins/test-amp-pixel.js

+4
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import {Services} from '#service';
12
import {installUrlReplacementsForEmbed} from '#service/url-replacements-impl';
23
import {VariableSource} from '#service/variable-source';
34

@@ -21,6 +22,7 @@ describes.realWin('amp-pixel', {amp: true}, (env) => {
2122
await createPixel(
2223
'https://pubads.g.doubleclick.net/activity;dc_iu=1/abc;ord=1?'
2324
);
25+
env.sandbox.spy(Services, 'urlReplacementsForDoc');
2426
});
2527

2628
function createPixel(src, referrerPolicy) {
@@ -170,6 +172,7 @@ describes.realWin('amp-pixel', {amp: true}, (env) => {
170172
'https://pubads.g.doubleclick.net/activity;dc_iu=1/abc;ord=1?ars=6'
171173
);
172174
expect(img.attributionSrc).to.equal('');
175+
expect(Services.urlReplacementsForDoc).to.be.calledWith(pixel);
173176
});
174177
});
175178

@@ -189,6 +192,7 @@ describes.realWin('amp-pixel', {amp: true}, (env) => {
189192
'https://pubads.g.doubleclick.net/activity;dc_iu=1/abc;ord=1?ars=6'
190193
);
191194
expect(img.attributionSrc).to.equal('https://adtech.example?ars=6');
195+
expect(Services.urlReplacementsForDoc).to.be.calledWith(pixel);
192196
});
193197
});
194198
});

0 commit comments

Comments
 (0)