Add GDPR/consent support to amp-slikeplayer#40500
Conversation
Integrate AMP consent framework so the Slike player iframe can request and receive consent data via postMessage, enabling cleo-player to handle personalisation and analytics decisions based on user consent state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds GDPR/consent-message support to amp-slikeplayer so the embedded Slike player iframe can request AMP consent data and receive it via postMessage, plus updates the example page and extends unit tests.
Changes:
- Add a
send-consent-datamessage handler inamp-slikeplayerand reply with AMP consent state viagetConsentDataToForward. - Update the Slikeplayer example page to include
amp-consentUI andamp-geoconfiguration. - Add unit tests covering consent request handling paths.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| extensions/amp-slikeplayer/0.1/amp-slikeplayer.js | Adds consent request handling and posts consent data back to the iframe. |
| extensions/amp-slikeplayer/0.1/test/test-amp-slikeplayer.js | Adds tests for consent request message handling and an iframe-lifecycle edge case. |
| examples/slikeplayer.amp.html | Demonstrates consent UI and geo grouping for the example page. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sendConsentData_() { | ||
| getConsentDataToForward(this.element, this.getConsentPolicy()).then( | ||
| (consents) => { | ||
| if (!this.iframe_ || !this.iframe_.contentWindow) { | ||
| return; | ||
| } | ||
| this.iframe_.contentWindow./*OK*/ postMessage( | ||
| { | ||
| 'sentinel': 'amp', | ||
| 'type': 'consent-data', | ||
| ...consents, | ||
| }, | ||
| this.targetOrigin_ | ||
| ); |
There was a problem hiding this comment.
sendConsentData_() uses this.targetOrigin_ as the postMessage targetOrigin, but targetOrigin_ is never defined anywhere in this class. This will result in the message being sent with an invalid/incorrect targetOrigin (often preventing delivery). Capture the request's origin from the triggering message event and use that as targetOrigin (or otherwise compute/store a validated origin when creating the iframe) instead of referencing an undefined field.
| it('calls sendConsentData_ on send-consent-data message', async () => { | ||
| const consentData = { | ||
| consentPolicyState: 1, | ||
| consentString: 'abc123', | ||
| consentMetadata: {gdprApplies: true, purposeOne: true}, | ||
| consentPolicySharedData: null, | ||
| }; | ||
| env.sandbox | ||
| .stub(consent, 'getConsentDataToForward') | ||
| .resolves(consentData); | ||
|
|
||
| const {iframe, impl} = await buildPlayer(); | ||
| const sendSpy = env.sandbox.spy(impl, 'sendConsentData_'); | ||
|
|
||
| // Simulate consent request from iframe (raw object, not JSON) | ||
| impl.onMessage_({ | ||
| source: iframe.contentWindow, | ||
| data: {type: 'send-consent-data', sentinel: 'amp'}, | ||
| }); | ||
|
|
||
| expect(sendSpy).to.have.been.calledOnce; | ||
|
|
||
| // Wait for the consent promise to resolve | ||
| await new Promise((r) => setTimeout(r, 0)); | ||
|
|
||
| expect(consent.getConsentDataToForward).to.have.been.calledOnce; | ||
| }); |
There was a problem hiding this comment.
This test only verifies that sendConsentData_ was invoked and that getConsentDataToForward was called, but it never asserts that consent data is actually forwarded to the iframe (e.g., that postMessage is called with {sentinel:'amp', type:'consent-data', ...} and the expected consent fields). Add an assertion around the postMessage call/payload so regressions in the response format or posting behavior are caught.
| env.sandbox | ||
| .stub(consent, 'getConsentDataToForward') | ||
| .resolves(consentData); | ||
|
|
||
| const {iframe, impl} = await buildPlayer(); | ||
|
|
||
| // Destroy iframe before consent resolves | ||
| impl.iframe_ = null; | ||
|
|
||
| impl.onMessage_({ | ||
| source: iframe.contentWindow, | ||
| data: {type: 'send-consent-data', sentinel: 'amp'}, | ||
| }); | ||
|
|
||
| await new Promise((r) => setTimeout(r, 0)); |
There was a problem hiding this comment.
The test name/comment says the iframe is destroyed before the consent promise resolves, but impl.iframe_ is set to null before onMessage_ is called. That means onMessage_ returns early and the async path in sendConsentData_ (the guard that checks this.iframe_ after consent data resolves) is never exercised. To cover the intended edge case, trigger the request first and then null out impl.iframe_ while the consent promise is still pending (e.g., with a manually controlled promise).
| env.sandbox | |
| .stub(consent, 'getConsentDataToForward') | |
| .resolves(consentData); | |
| const {iframe, impl} = await buildPlayer(); | |
| // Destroy iframe before consent resolves | |
| impl.iframe_ = null; | |
| impl.onMessage_({ | |
| source: iframe.contentWindow, | |
| data: {type: 'send-consent-data', sentinel: 'amp'}, | |
| }); | |
| await new Promise((r) => setTimeout(r, 0)); | |
| let resolveConsentData; | |
| const consentDataPromise = new Promise((resolve) => { | |
| resolveConsentData = resolve; | |
| }); | |
| env.sandbox | |
| .stub(consent, 'getConsentDataToForward') | |
| .returns(consentDataPromise); | |
| const {iframe, impl} = await buildPlayer(); | |
| impl.onMessage_({ | |
| source: iframe.contentWindow, | |
| data: {type: 'send-consent-data', sentinel: 'amp'}, | |
| }); | |
| // Destroy iframe while consent is still pending so the async guard | |
| // inside sendConsentData_() is exercised after the promise resolves. | |
| impl.iframe_ = null; | |
| resolveConsentData(consentData); | |
| await Promise.resolve(); |
| <!-- Geo detection for consent --> | ||
| <amp-geo layout="nodisplay"> | ||
| <script type="application/json"> | ||
| { | ||
| "ISOCountryGroups": { | ||
| "eea": ["at", "be", "bg", "hr", "cy", "cz", "dk", "ee", "fi", "fr", | ||
| "de", "gr", "hu", "ie", "it", "lv", "lt", "lu", "mt", "nl", | ||
| "pl", "pt", "ro", "sk", "si", "es", "se", "gb", "is", "li", | ||
| "no", "ch"] | ||
| } | ||
| } | ||
| </script> | ||
| </amp-geo> | ||
|
|
||
| <!-- Consent management --> | ||
| <amp-consent id="slike-consent" layout="nodisplay"> | ||
| <script type="application/json"> | ||
| { | ||
| "consentInstanceId": "slike-player-consent", | ||
| "consentRequired": true, | ||
| "promptUI": "consent-prompt", | ||
| "postPromptUI": "post-consent-prompt" | ||
| } |
There was a problem hiding this comment.
amp-geo is configured with an eea group, but the amp-consent config doesn't reference any geo group (e.g., via promptIfUnknownForGeoGroup / consentRequired gating), so geo detection has no effect and the consent UI will always be required. Either wire amp-consent to the eea group as described in the PR summary, or remove the unused amp-geo configuration from the example.
Summary
amp-slikeplayerso the embedded Slike player iframe can request and receive consent data via postMessageamp-consentandamp-geoto the example page with a consent banner UIChanges
getConsentDataToForward, listen forsend-consent-datamessages from iframe, respond with AMP consent state (consentPolicyState,consentString,consentMetadata,consentPolicySharedData)amp-consent+amp-geoscripts, consent prompt UI, and geo-based EEA country groupsHow it works
{ type: 'send-consent-data', sentinel: 'amp' }to parentamp-slikeplayerreceives this, queries AMP's consent service{ sentinel: 'amp', type: 'consent-data', consentPolicyState, ... }