-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Defining userSync.filterSettings.iframe only disables image syncing in v3.7.1 #4864
Comments
After having dug around a bit I think this issue stems from the removal of the old Output of before https://github.com/prebid/Prebid.js/blob/master/src/config.js#L298 I get the following: {
auctionDelay: 0
filterSettings: {
image: Object { bidders: "*", filter: "include" }
}
syncDelay: 3000
syncEnabled: true
syncsPerBidder: 5
} The value of {
auctionDelay: 0
filterSettings: {
iframe: Object { bidders: "*", filter: "include" }
}
syncDelay: 3000
syncEnabled: true
syncsPerBidder: 5
} So either there needs to be no more default, a default another way or that code needs to be expanded to check for objects and merge their keys. Please advise as to which is preferable. Personally I think the logic on that line should be changed to a "merge" like deepmerge as there maybe other options/things that need to be merged down the line. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Wanted to bump this and make sure it is not over looked. |
@Austinb Thanks for the bump & reporting it - I'll take a look at this. |
Type of issue
Bug or undocumented change in default behavior when moving from v2.x to v.3.x
Description
In version 3.7.1 (and 3.3.0) when you specify the
userSync.filterSettings
it overwrites the defaults rather than being merged with the defaults. If you define theuserSync.filterSettings.iframe
settings the defaultuserSync.filterSettings.image
settings from https://github.com/prebid/Prebid.js/blob/master/src/userSync.js#L9 are lost. This effectively disables image pixel syncing unless you define it explicitly or use theuserSync.filterSettings.all
property.In the v2.x series it did not do this (verified in both v2.39.0 and v2.34.0 with currently active deployments).
Steps to reproduce
If you have your pb.js config defined with the following
The response from the
config.getConfig
calls in https://github.com/prebid/Prebid.js/blob/master/src/adapters/bidderFactory.js#L335 only contains the defined settings above. As a result the value ofpixelEnabled
isfalse
a couple of lines down.Test page
Not available
Expected results
If the image syncing is supposed to be enabled by default then it should always be enabled unless otherwise explicitly disabled in the
userSync.filterSettings.image
key.Actual results
If the
userSync.filterSettings.image
setting is not defined when defininguserSync.filterSettings.iframe
image syncs are disabled.Platform details
Building Prebis.js on:
Ubuntu 18.04.3
Node JS v10.17.0
Npm 6.13.6
Other information
Did a quick search and was unable to find any specific change that affected this but I did not spend a lot of time searching either. I did find some comments in https://github.com/prebid/Prebid.js/pull/4241/files referencing that the
permittedPixels.image
should remaintrue
by default which leads me to believe this is a bug and not an undocumented change in default behavior. If I have some time I will step through and see what change broke the default behavior.The text was updated successfully, but these errors were encountered: