Skip to content

Commit

Permalink
Remove Engadget workaround from google-ima shim (#36)
Browse files Browse the repository at this point in the history
There was a workaround in the google-ima shim that applied only to
www.engadget.com that set a current advert to be returned by
AdEvent.getAd() and AdsManager.getCurrentAd(). When testing the
website, I couldn't see that code-path being hit at all, so let's
remove the workaround. Also, let's take care to return null instead of
undefined as is specified[1].

In the future, we could expand this logic if necessary. I think
setting the currentAd to new Ad() when the AdEvent.Type.LOADED event
fires and back to null when AdEvent.Type.ALL_ADS_COMPLETED fires would
make sense - but we would need to test that doesn't cause more website
breakage first.

1 - https://developers.google.com/interactive-media-ads/docs/sdks/html5/client-side/reference/js/google.ima.AdEvent#getAd
  • Loading branch information
kzar authored Nov 30, 2023
1 parent 6b7c7be commit acf80a1
Showing 1 changed file with 4 additions and 16 deletions.
20 changes: 4 additions & 16 deletions surrogates/google-ima.js
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ if (!window.google || !window.google.ima || !window.google.ima.VERSION) {
}
}

// eslint-disable-next-line no-unused-vars
class Ad {
constructor() {
this._pi = new AdPodInfo();
Expand Down Expand Up @@ -650,22 +651,9 @@ if (!window.google || !window.google.ima || !window.google.ima.VERSION) {
AdError.ErrorCode = {};
AdError.Type = {};

const isEngadget = () => {
try {
for (const ctx of Object.values(window.vidible._getContexts())) {
const player = ctx.getPlayer();
if (!player) { continue; }
const div = player.div;
if (!div) { continue; }
if (div.innerHTML.includes("www.engadget.com")) {
return true;
}
}
} catch (_) {}
return false;
};

const currentAd = isEngadget() ? undefined : new Ad();
// TODO: Consider setting this to `new Ad()` when AdEvent.Type.LOADED fires
// and clearing it again after AdEvent.Type.ALL_ADS_COMPLETED fires.
const currentAd = null;

class AdEvent {
constructor(type) {
Expand Down

0 comments on commit acf80a1

Please sign in to comment.