Prebid 11: add adUnit.element option

[dgirardi]

Type of change

• Feature

Description of change

• add a element option to ad units that can be used to associate it with a DOM element
• add a getAdUnitElement(adUnit | bidRequest | bidResponse) utility function that returns the DOM element associated with a given ad unit / bid request / bid response
• if adUnit.element is not specified, defaults to getElementById(adUnit.code)
• refactor most instances of the pattern document.getElementById(adUnit.code) into getAdUnitElement(adUnit). In some cases the logic was too complex for a simple refactor; I noted those with a TODO comment

Other information

Related: #14408 and #13125

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d7932f800

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[coveralls]

Pull Request Test Coverage Report for Build 22200455621

Details

• 119 of 142 (83.8%) changed or added relevant lines in 50 files are covered.
• 428 unchanged lines in 44 files lost coverage.
• Overall coverage increased (+0.01%) to 96.215%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
modules/bidViewabilityIO.js	0	1	0.0%
modules/fanBidAdapter.js	0	1	0.0%
modules/ixBidAdapter.js	5	6	83.33%
modules/medianetBidAdapter.js	3	4	75.0%
modules/nexverseBidAdapter.js	0	1	0.0%
modules/pubmaticBidAdapter.js	0	1	0.0%
modules/adrelevantisBidAdapter.js	0	2	0.0%
modules/appnexusBidAdapter.js	0	5	0.0%
modules/mediafuseBidAdapter.js	0	5	0.0%
modules/msftBidAdapter.js	0	5	0.0%

Files with Coverage Reduction	New Missed Lines	%
modules/adoceanBidAdapter.js	1	96.97%
modules/undertoneBidAdapter.js	1	97.89%
modules/visxBidAdapter.js	1	94.86%
test/spec/modules/id5AnalyticsAdapter_spec.js	1	96.13%
test/spec/modules/trafficgateBidAdapter_spec.js	1	98.33%
modules/asteriobidAnalyticsAdapter.js	2	79.47%
modules/AsteriobidPbmAnalyticsAdapter.js	2	80.41%
modules/invisiblyAnalyticsAdapter.js	2	97.87%
modules/medianetBidAdapter.js	2	92.38%
modules/prebidServerBidAdapter/index.ts	2	93.12%

Totals
Change from base Build 21839663904:	0.01%
Covered Lines:	208120
Relevant Lines:	216307

---

💛 - Coveralls

[monis0395]

Missed this, will start review this tomorrow

[monis0395]

@dgirardi Is the GPT slot fallback for getAdUnitElement intentionally deferred to a follow-up PR (per the TODO on line 5-6 of src/utils/adUnits.ts)? Several adapters still independently implement GPT fallback after calling getAdUnitElement (33across, ix, visx, underdogmedia, valuad, fan, medianet), so it'd be great to confirm this is planned for unification in a subsequent PR.

Also a lot of adapters can use getAdUnitElement(adUnitCode) for replacement of document.getElementById(bidReq.adUnitCode) is that also intentionally left ?

[patmmccann]

@dgirardi Is the GPT slot fallback for getAdUnitElement intentionally deferred to a follow-up PR (per the TODO on line 5-6 of src/utils/adUnits.ts)? Several adapters still independently implement GPT fallback after calling getAdUnitElement (33across, ix, visx, underdogmedia, valuad, fan, medianet), so it'd be great to confirm this is planned for unification in a subsequent PR.

Also a lot of adapters can use getAdUnitElement(adUnitCode) for replacement of document.getElementById(bidReq.adUnitCode) is that also intentionally left ?

is the todo addressed in #14480 ? That's based on this

[dgirardi]

Is the GPT slot fallback for getAdUnitElement intentionally deferred to a follow-up PR (per the TODO on line 5-6 of src/utils/adUnits.ts)?

Currently #14480 does not attempt to unify them and I'm not sure it will, so I removed the TODO for now. It's still an option if we decide to attempt it, but since writing that comment I realized it's a much bigger task than we were originally signing up for.

There's quite a few adapters that have surprisingly complicated logic; I'd love to simplify it but it's also not clear that doing so would benefit publishers enough, compared to the risk of some adapters picking up the new unified logic as "your viewability is now lower".

Also a lot of adapters can use getAdUnitElement(adUnitCode) for replacement of document.getElementById(bidReq.adUnitCode) is that also intentionally left ?

The goal of this PR is to replace the latter with the former. I left some of the "old" use in when the replacement would not have been functionally equivalent, and added a TODO comment in those cases. Is that what you're referring to? If not, it's not intentional, please do point out what I missed.

[monis0395]

Is the GPT slot fallback for getAdUnitElement intentionally deferred to a follow-up PR (per the TODO on line 5-6 of src/utils/adUnits.ts)?

Currently #14480 does not attempt to unify them and I'm not sure it will, so I removed the TODO for now. It's still an option if we decide to attempt it, but since writing that comment I realized it's a much bigger task than we were originally signing up for.

Makes sense. Agreed, it’s a much bigger task.

One question: is #14480 branched off this PR? I assumed it would be. Ideally #14480 should build on top of this PR’s getAdUnitElement rather than exist as a sibling, to avoid conflicts and to ensure the GPT unification uses the new element resolution path.

There's quite a few adapters that have surprisingly complicated logic; I'd love to simplify it but it's also not clear that doing so would benefit publishers enough, compared to the risk of some adapters picking up the new unified logic as "your viewability is now lower".

Correct

Also a lot of adapters can use getAdUnitElement(adUnitCode) for replacement of document.getElementById(bidReq.adUnitCode) is that also intentionally left ?

The goal of this PR is to replace the latter with the former. I left some of the "old" use in when the replacement would not have been functionally equivalent, and added a TODO comment in those cases. Is that what you're referring to? If not, it's not intentional, please do point out what I missed.

Examples I noticed

libraries/placementPositionInfo/placementPositionInfo.js line 52:

• This still uses selfWin.document.getElementById(bidReq.adUnitCode) directly. Since bidReq is available here, could this use getAdUnitElement(bidReq) as well?

modules/pubmaticBidAdapter.js line 717:

• addViewabilityToImp still uses raw document.getElementById(adUnitCode). The bid request is available at the call site, so the signature could be updated to accept it.