Skip to content
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

Add AdRequest.omidAccessModeRules property to google-ima shim #3911

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kzar
Copy link
Contributor

@kzar kzar commented Nov 28, 2023

Add the missing AdRequest.omidAccessModeRules[1] property, to avoid
breaking websites that access it.

1 - https://developers.google.com/interactive-media-ads/docs/sdks/html5/client-side/reference/js/google.ima.AdsRequest#omidAccessModeRules

@kzar
Copy link
Contributor Author

kzar commented Nov 28, 2023

@gorhill if you'd like to test this, browse to https://www.corriere.it/video-articoli/2022/07/13/missione-wwf-liberare-mare-plastica/9abb64de-029d-11ed-a0cc-ad3c68cacbae.shtml (making sure google-ima.js is being redirected to the shim) and try playing the video. Without the fix, I found the video wouldn't play and an error showed up in the console about the missing property.

@gorhill
Copy link
Owner

gorhill commented Nov 28, 2023

On my side, the video still doesn't play with uBO default lists the filter ||imasdk.googleapis.com/js/sdkloader/ima3.js$script,redirect-rule=google-ima.js:10,from=corriere.it and the fix.

What works to unbreak the video on our side is *$script,redirect-rule=noopjs,from=corriere.it for that site, or simply blocking ||imasdk.googleapis.com^ without redirection.

@kzar
Copy link
Contributor Author

kzar commented Nov 28, 2023

Dang OK, yea there's quite a lot going on in that website. It's probably not the best test case, but it's the only one I have. In the DuckDuckGo Privacy Essentials extension, this change fixed video playback. But there we are also blocking and redirecting a bunch of other requests. Reproducing the issue to see it being fixed here might be a PITA, sorry :/.

FWIW I am quite confident in the change though, the website attempts to do t.omidAccessModeRules[google.ima.OmidVerificationVendor.OTHER]=google.ima.OmidAccessMode.FULL with t being an AdRequest in 4.min.js. The AdRequest.omidAccessModeRules property is also listed in the docs.

But yea, no pressure if you'd rather not merge. Just wanted to share this back! Sorry I don't have a good test website.

@gorhill
Copy link
Owner

gorhill commented Nov 28, 2023

I will investigate more when I get the time to find out where it fails on my side.

@kzar
Copy link
Contributor Author

kzar commented Nov 28, 2023

OK cool, sounds good. If it helps, here's a (compressed) HAR file showing which requests the DuckDuckGo extension redirected/blocked/allowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants