gorhill / uBlock

uBlock Origin - An efficient blocker for Chromium and Firefox. Fast and lean.
GNU General Public License v3.0
44.49k stars 2.99k forks source link

Further improve google-ima shim script #3900

Closed kzar closed 8 months ago

kzar commented 8 months ago

I worked through some of the websites listed in the google-ima shim script issue[1], to see what was going wrong. It turned out the addEventListener method supports an optional context Object, which is bound to the listener if provided. Some websites make use of that, and then break when this is not bound correctly when events are dispatched.

See also https://github.com/duckduckgo/tracker-surrogates/pull/24

1 - https://github.com/uBlockOrigin/uBlock-issues/issues/2265

kzar commented 8 months ago

This has not gone through review yet for the DuckDuckGo extension, so check you're happy with the changes. But it should get the trailer video playing for https://gem.cbc.ca/push .

gorhill commented 8 months ago

All good for me, the video played fine with the changes. I noticed once in a while an error reported in the console as a result of an installed listener throwing, but the video still played fine.

TypeError: Cannot read properties of undefined (reading 'podIndex')
    at t.value (c93403b3.687f35139efea38b.js:1:613175)
    at t.value (c93403b3.687f35139efea38b.js:1:610187)
    at AdsManager._dispatch (ima3.js:342:11)
    at ima3.js:469:18
kzar commented 8 months ago

All good for me, the video played fine with the changes. I noticed once in a while an error reported in the console as a result of an installed listener throwing, but the video still played fine.

TypeError: Cannot read properties of undefined (reading 'podIndex')
    at t.value (c93403b3.687f35139efea38b.js:1:613175)
    at t.value (c93403b3.687f35139efea38b.js:1:610187)
    at AdsManager._dispatch (ima3.js:342:11)
    at ima3.js:469:18

Yea, I saw that one too. It seems that the website expects some of the event Objects to have an adPodInfo property. I nearly added it, but like you say it didn't seem to matter here + the docs don't mention that property being present. Maybe I'm missing something.