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

Reset hb_* keys only for registered adunits #902

Closed
wants to merge 1 commit into from

Conversation

darbh
Copy link

@darbh darbh commented Jan 3, 2017

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other

Description of change

  • test parameters for validating bids
{
  bidder: '<bidder name>',
  params: {
    // ...
  }
}
  • contact email of the adapter’s maintainer
  • official adapter submission

Other information

Copy link
Collaborator

@matthewlane matthewlane left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, a few changes requested below

slot.setTargeting(key,null);
// Reset Only Registerd dfp-adunits
$$PREBID_GLOBAL$$.adUnits.find(function (unit) {
if(unit.code === slot.getSlotElementId()){
Copy link
Collaborator

Choose a reason for hiding this comment

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

unit.code should also check against slot.getAdUnitPath(). See https://github.com/prebid/Prebid.js/blob/master/src/prebid.js#L124 for a similar example

@@ -113,7 +113,12 @@ function resetPresetTargeting() {
if (isGptPubadsDefined()) {
window.googletag.pubads().getSlots().forEach(slot => {
pbTargetingKeys.forEach(function(key){
slot.setTargeting(key,null);
// Reset Only Registerd dfp-adunits
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use spaces rather than tabs in this section to match the rest of the file

@matthewlane matthewlane added this to the Prebid 0.18.0 milestone Jan 6, 2017
@matthewlane
Copy link
Collaborator

@darbh Any updates available? I'd like to get this into the next release

@matthewlane
Copy link
Collaborator

This has been updated and rebased in #934

@darbh
Copy link
Author

darbh commented Jan 31, 2017 via email

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

Successfully merging this pull request may close these issues.

3 participants