Skip to content

Refactor: Remove master/slave terminology from window.context API (Fi… #40365

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

itzperi
Copy link

@itzperi itzperi commented Aug 4, 2025

…xes #30789)

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors the window.context API to eliminate problematic master/slave terminology, replacing it with primary/secondary terminology while maintaining full backward compatibility.

Key changes include:

  • Function and property renames from master/slave to primary/secondary terminology
  • Addition of legacy compatibility properties and methods
  • Comprehensive test updates to use the new terminology

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/unit/test-3p.js Updates test cases to use computeInPrimaryFrame and primary/secondary terminology
test/unit/ads/test-ssp.js Updates SSP ad tests to use new primary frame terminology
test/unit/ads/test-pubmine.js Updates pubmine ad tests to use isPrimary property
test/unit/3p/test-ampcontext-integration.js Updates integration tests to use primaryFrameSelection
test/integration/test-amp-ad-3p.js Updates integration tests to expect new context properties
src/3p-frame.js Updates comment to use primary frame terminology
extensions/amp-ad/0.1/test/test-amp-ad-xorigin-iframe-handler.js Updates iframe handler tests to use primary terminology
extensions/amp-ad/0.1/amp-ad-xorigin-iframe-handler.js Updates iframe handling logic to check for primary frames
build-system/externs/amp.extern.js Adds new primary properties while preserving legacy ones
ads/vendors/*.js Updates vendor implementations to use new primary frame API
WINDOW_CONTEXT_API_REFACTORING.md Documentation of the refactoring effort
3p/ampcontext.js Updates context implementation comments
3p/ampcontext-integration.js Core implementation of new primary frame API with legacy compatibility
3p/3p.js Implementation of computeInPrimaryFrame with legacy wrapper

Comment on lines +24 to 25
global.context.primary.Swoop.announcePlace(global, data);
}
Copy link
Preview

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

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

The indentation is inconsistent - this line appears to have extra spaces compared to the surrounding code structure.

Suggested change
global.context.primary.Swoop.announcePlace(global, data);
}
global.context.primary.Swoop.announcePlace(global, data);
}

Copilot uses AI. Check for mistakes.

@erwinmombay
Copy link
Member

heya our expert in the ads space is currently on leave and I don't feel comfortable approving this without their review, so it might take a bit for to get to this review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants