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

More improvements to the google-ima shim script #3908

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

kzar
Copy link
Contributor

@kzar kzar commented Nov 16, 2023

We have enabled the google-ima shim script again in the DuckDuckGo
Privacy Essentials browser extension, and found a couple more issues:

  • Some websites set the enablePreloading[1] option, which should
    cause[2] the AdsManager.init() method to trigger the LOADED AdEvent
    to fire. If the event doesn't fire, those websites can get stuck
    waiting for the event forever.
  • When AdsManager.start() method is called, a bunch of events are
    dispatched in order, to simulate ads loading, playing and
    finishing. There was a mistake in that logic though. The
    CONTENT_PAUSE_REQUESTED and CONTENT_RESUME_REQUESTED events[3]
    should fire as the ads start and finish respectively. By firing the
    latter early, and skipping the former, some websites got confused
    and tried to display ad overlays at the same time as playing their
    content, or didn't display they content at all.

1 - https://developers.google.com/interactive-media-ads/docs/sdks/html5/client-side/reference/js/google.ima.AdsRenderingSettings#enablePreloading
2 - https://developers.google.com/interactive-media-ads/docs/sdks/html5/client-side/preload#timing
3 - https://developers.google.com/interactive-media-ads/docs/sdks/html5/client-side/reference/js/google.ima.AdEvent

We have enabled the google-ima shim script again in the DuckDuckGo
Privacy Essentials browser extension, and found a couple more issues:
- Some websites set the enablePreloading[1] option, which should
  cause[2] the AdsManager.init() method to trigger the LOADED AdEvent
  to fire. If the event doesn't fire, those websites can get stuck
  waiting for the event forever.
- When AdsManager.start() method is called, a bunch of events are
  dispatched in order, to simulate ads loading, playing and
  finishing. There was a mistake in that logic though. The
  CONTENT_PAUSE_REQUESTED and CONTENT_RESUME_REQUESTED events[3]
  should fire as the ads start and finish respectively. By firing the
  latter early, and skipping the former, some websites got confused
  and tried to display ad overlays at the same time as playing their
  content, or didn't display they content at all.

1 - https://developers.google.com/interactive-media-ads/docs/sdks/html5/client-side/reference/js/google.ima.AdsRenderingSettings#enablePreloading
2 - https://developers.google.com/interactive-media-ads/docs/sdks/html5/client-side/preload#timing
3 - https://developers.google.com/interactive-media-ads/docs/sdks/html5/client-side/reference/js/google.ima.AdEvent
@kzar kzar force-pushed the improve-google-ima-shim-again branch from 622f5b5 to 084e38f Compare November 16, 2023 12:26
@kzar kzar marked this pull request as ready for review November 16, 2023 12:27
@kzar
Copy link
Contributor Author

kzar commented Nov 16, 2023

@gorhill A couple more improvements to the google-ima shim for you. If you'd like to test:

  1. Try playing the AI audio version of articles on rawstory.com with the Trinity Audio widget.
  2. Try playing a video included with one of the articles on metro.co.uk

(Making sure the google-ima shim is active obviously!)

@gorhill
Copy link
Owner

gorhill commented Nov 16, 2023

Yes, worked. For metro.co.uk, it seems a bit difficult to reproduce but I did get the hung video with spinning wheel once. For rawstory.com, I get the hung player. Incidentally, wholly blocking ima3.js on rawstory.com also solves the issue and it also removes the introduction about sponsor.

@gorhill gorhill merged commit c1d8f59 into gorhill:master Nov 16, 2023
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