Defining userSync.filterSettings.iframe only disables image syncing in v3.7.1

[Austinb]

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 the userSync.filterSettings.iframe settings the default userSync.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 the userSync.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

{
    userSync: {
        filterSettings: {
            iframe: {
                bidders: '*', 
                filter: 'include',
            },
        },
    },
}

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 of pixelEnabled is false 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 defining userSync.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 remain true 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.

[Austinb]

After having dug around a bit I think this issue stems from the removal of the old userSync.pixelEnabled value in #4580. Before it appears the library was always using that value at https://github.com/prebid/Prebid.js/blob/master/src/adapters/bidderFactory.js#L337. Now since that setting is gone the default is not working properly because the Object.assign() at https://github.com/prebid/Prebid.js/blob/master/src/config.js#L298 is (and has been) overwriting the userSync.filterSettings key. That key already exists in the defaults from https://github.com/prebid/Prebid.js/blob/master/src/userSync.js#L6 and is not being merged.

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 option after that line is as follows:

{
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.

[stale]

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.

[Austinb]

Wanted to bump this and make sure it is not over looked.

[jsnellbaker]

@Austinb Thanks for the bump & reporting it - I'll take a look at this.