Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

move logic to check if CMP is not found #2715

Merged
merged 1 commit into from
Jun 21, 2018
Merged

Conversation

jsnellbaker
Copy link
Collaborator

Type of change

  • Bugfix
  • Refactoring (no functional changes, no api changes)

Description of change

To address the issue reported in #2714

With the old logic, the consentManagement module would go through part of the iframe workflow to detect if the CMP locator frame was not found. While this code was part of the IAB spec for the CMP, in our case it meant that a local version of the window.__cmp function was still created in a page where a normal CMP doesn't exist (which was the issue that was reported).

This fix moves the check that ultimately detects if the CMP locator frame is present so that it's outside the iframe function in order to prevent creating the window.__cmp function) while still throwing an error.

@bretg bretg requested a review from snapwich June 11, 2018 21:34
@jsnellbaker jsnellbaker requested a review from mkendall07 June 18, 2018 17:58
@harpere harpere added needs review needs 2nd review Core module updates require two approvals from the core team labels Jun 20, 2018
Copy link
Collaborator

@snapwich snapwich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@snapwich snapwich removed the needs 2nd review Core module updates require two approvals from the core team label Jun 21, 2018
@snapwich snapwich merged commit 4723710 into master Jun 21, 2018
@mkendall07 mkendall07 deleted the move_cmp-not-found_logic branch August 17, 2018 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants