KEMBAR78
Core: remove use of document.write in rendering by dgirardi · Pull Request #13851 · prebid/Prebid.js · GitHub
Skip to content

Conversation

@dgirardi
Copy link
Collaborator

@dgirardi dgirardi commented Sep 4, 2025

Type of change

  • Bugfix

Description of change

This removes the special case for bids providing ad (and not adUrl) that, when not in a safeframe, are rendered using document.write; replacing it with an iframe srcdoc instead (the same logic used in the safeframe case).

From my testing this fixes the GAM viewability issue reported in #13836 without the need to turn off yielding.

Other information

Related: #8337

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 17469486843

Details

  • 13 of 14 (92.86%) changed or added relevant lines in 2 files are covered.
  • 59 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+33.1%) to 96.253%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/adRendering.ts 5 6 83.33%
Files with Coverage Reduction New Missed Lines %
src/adloader.js 12 78.57%
src/utils.js 47 89.07%
Totals Coverage Status
Change from base Build #3: 33.1%
Covered Lines: 196572
Relevant Lines: 204224

💛 - Coveralls

e?.stack && logError(e);
}
);
// TODO: this is almost certainly the wrong way to do this
Copy link
Collaborator

Choose a reason for hiding this comment

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

haha

Copy link
Collaborator

@robertrmartinez robertrmartinez left a comment

Choose a reason for hiding this comment

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

looks good

Once I pick back up testing and we have merged all these things I foresee perhaps slowly trying to pick out each thing to determine what really fixes our issues.

@patmmccann patmmccann merged commit 48419a6 into master Sep 11, 2025
20 checks passed
@patmmccann patmmccann deleted the gam-viewable branch September 11, 2025 13:21
@dgirardi dgirardi mentioned this pull request Sep 29, 2025
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.

7 participants