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

Discovery Bid Adapter : fix window.top bug #11511

Merged
merged 3 commits into from
Jun 13, 2024
Merged

Conversation

lhxx121
Copy link
Contributor

@lhxx121 lhxx121 commented May 17, 2024

Type of change

  • Bugfix

  • Feature

  • New bidder adapter

  • Updated bidder adapter

  • Code style update (formatting, local variables)

  • Refactoring (no functional changes, no api changes)

  • Build related changes

  • CI related changes

  • Does this change affect user-facing APIs or examples documented on http://prebid.org?

  • Other

Description of change

Fix bugs caused by Same-Origin Policy

Other information

@ChrisHuie ChrisHuie changed the title Discovery Bid Adapter: Fix window.top bug Discovery Bid Adapter : fix window.top bug May 21, 2024
@ChrisHuie ChrisHuie requested a review from Rothalack May 21, 2024 15:15
@lhxx121
Copy link
Contributor Author

lhxx121 commented May 30, 2024

@ChrisHuie Hi, sorry to bother you, could you please help me cr it? This submission is very important to us.❤️

export function getDM(win = window) {
let dm;
try {
dm = win.top.navigator.deviceMemory;
Copy link
Collaborator

Choose a reason for hiding this comment

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

See #11001 ; please move these functions into a library in the fpd group of imports. I don't think we want them in everyone's device object by default though, as the primary purpose is fingerprinting, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have modified it as required, please help me cr it again.
Thanks.

@Rothalack
Copy link
Collaborator

The code changes look good, but I am unable to checkout this pr and test. I think you need to rebase master. It doesn't make sense to me why, these changes shouldn't conflict, but I don't want to try to rebase or change anything on my end to get this working.

@lhxx121
Copy link
Contributor Author

lhxx121 commented Jun 11, 2024

@Rothalack I understand. I will modify and submit immediately

Copy link
Collaborator

@Rothalack Rothalack left a comment

Choose a reason for hiding this comment

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

I still had to manually check this out into a new branch for myself to test. Everything is working in testing. I still don't know why github complains about this when I try to gh pr checkout. I think it's because there's some divergence somewhere in your commit history that makes the simple gh checkout not work. I don't see any reason why that divergence is going to cause any issues. There's no conflicts. So I think we can just ignore it and merging this will work fine.

@patmmccann patmmccann merged commit 7ded2b7 into prebid:master Jun 13, 2024
4 checks passed
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.

None yet

4 participants