Skip to content

Commit

Permalink
🐛 Force transfer of amp-consent element to the FixedLayer (amppro…
Browse files Browse the repository at this point in the history
…ject#36223)

Fixes ampproject#36063

`amp-consent` explicitly adds itself to the `FixedLayer`, transferring itself before the iframe is loaded.

On a later pass, `FixedLayer` decides that `amp-consent` is not transferrable, so it returns it to the original `<body>` element.

Enabling `forceTransfer` causes the return reparenting to not occur, thus preventing the iframe from loading a second time.
  • Loading branch information
alanorozco authored and samouri committed Oct 7, 2021
1 parent a1f57f6 commit c6a56d5
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
7 changes: 5 additions & 2 deletions extensions/amp-consent/0.1/consent-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,11 @@ export class ConsentUI {
const {classList} = this.parent_;
classList.add('amp-active');
classList.remove('amp-hidden');
// Add to fixed layer
this.baseInstance_.getViewport().addToFixedLayer(this.parent_);

this.baseInstance_
.getViewport()
.addToFixedLayer(this.parent_, /* forceTransfer */ true);

if (this.isCreatedIframe_) {
// show() can be called multiple times, but notificationsUiManager
// ensures that only 1 is shown at a time, so no race condition here
Expand Down
18 changes: 12 additions & 6 deletions extensions/amp-consent/0.1/test/test-consent-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describes.realWin(
let ampdoc;
let consentUI;
let mockInstance;
let mockViewport;
let parent;
let ownersStubs;

Expand All @@ -46,18 +47,17 @@ describes.realWin(
postPrompt.setAttribute('id', 'testPost');
parent.appendChild(postPrompt);
doc.body.appendChild(parent);
mockViewport = {
addToFixedLayer: env.sandbox.spy(),
removeFromFixedLayer: () => {},
};
mockInstance = {
getAmpDoc: () => {
return ampdoc;
},
element: parent,
win,
getViewport: () => {
return {
addToFixedLayer: () => {},
removeFromFixedLayer: () => {},
};
},
getViewport: () => mockViewport,
getVsync: () => {
return {
mutate: (callback) => {
Expand Down Expand Up @@ -106,6 +106,12 @@ describes.realWin(
const consentUI = new ConsentUI(mockInstance, config);
const showIframeSpy = env.sandbox.spy(consentUI, 'showIframe_');
consentUI.show(false);
expect(
mockViewport.addToFixedLayer.withArgs(
mockInstance.element,
/* forceTransfer */ true
)
).to.have.been.calledOnce;
consentUI.iframeReady_.resolve();
return whenCalled(showIframeSpy).then(() => Promise.resolve(consentUI));
};
Expand Down

0 comments on commit c6a56d5

Please sign in to comment.