pinterest / widgets

JavaScript widgets, including the Pin It button.
Other
211 stars 88 forks source link

Unknown blur event is overwriting video element classes #102

Closed ETNOL closed 3 years ago

ETNOL commented 3 years ago

👋 from Mediavine

Our publishers recently started experiencing issues with video playback after blur events. I found a few lines in pinit_main.js that appear to be overwriting the className prop on our video element, causing videojs to crash. Weird thing is, the code doesn't align with this repo, as far as I can tell. See below:

image (3)

This is being served currently from https://assets.pinterest.com/js/pinit_main.js

Any ideas?

Thanks! Nolte @ Mediavine

ETNOL commented 3 years ago

You're probably able to track these breadcrumbs faster than me, but I found the tv: ${version} value of the public build set to 2021020401, Feb 04, 2021, while this repo reflects Dec. 7th 2020.

pieisgood commented 3 years ago

Piling on here, would really like this addressed, @kentbrew

gorenburg commented 3 years ago

another issue here that if the video is playing in the tab, you switch the tab to continue listening to the tab in background - it just pauses the video

here is the video of the issue showcasing both classes overrides and pausing the video: https://monosnap.com/file/IixdyBJGpaxXrCdqzGwRU8XDt3BlOT

pieisgood commented 3 years ago

If the code in this repo was up to date I'd fix it myself and put in a pull request, but as it stands we can't do anything.

grajzer commented 3 years ago

Come on Pinterest guys, this is highly unprofessional. Your script aggressively messes up all video elements on a page and you don't even bother to do anything for almost two weeks?

gorenburg commented 3 years ago

On Feb 8th I've messaged their support (Ticket: 294205). They've asked a bunch of questions instead of watching the video with the issue and then they just disappeared. Looks like they don't care. Maybe we all should start removing their widget?

doctafaustus commented 3 years ago

Confirmed this is an issue for us as well. Looks like any site using this script will have the same problem. Not sure why this repo's code doesn't match up. In any case, it looks like someone may have used = when they should have used == or ===.

if (b[c].className = e.a.k + "_video") {
doctafaustus commented 3 years ago

The answer I got from Pinterest's business support, after sending a video and referring them to this issue:

I would recommend you to once check with your web developer since they will be able to detect what's going wrong with this. This seems to be an issue with the website.

😐

kentbrew commented 3 years ago

Fixed, landing shortly. Very sorry about this; entirely my fault.

gorenburg commented 3 years ago

@kentbrew pausing the video still brings the issues. we have online courses and some users prefer to leave the tab in the background and continue listening to the course. Your widget will pause the video. Why you interfere with user's websites? If you'll keep pausing the videos - we might entirely remove your widget (currently, we've removed it from a great number of pages). You're also pausing video is sliders (same side effect from 'blur' event). Sharing widget should be a 'sharing widget' - it should not modify the user's web-site

We receive support tickets, not pinterest since users don't know that pinterest's widget is making their experience inconvinient

kentbrew commented 3 years ago

May I have an URL to test, please?

tombigel commented 3 years ago

@kentbrew we are experiencing this behavior also on blogs created with Wix.com - On our sites muted videos are paused while the page is unfocused, and play again when the page regains focus. The Pinterest widget is interfering with this behavior, videos not playing again and and users complain. As @gorenburg said - Please let the share widget do the one thing it is supposed to do and no more.

kentbrew commented 3 years ago

The fix I landed Friday should have taken care of this. May I please see an URL where the bug is still present? I have a test page at https://kentbrew.neocities.org/video-pause-test/ and I'm not seeing it.

tombigel commented 3 years ago

@kentbrew sorry about that, after another refresh this actually seems fixed. Will let QA double check me before I celebrate though :), thank you!

kentbrew commented 3 years ago

That's great, thanks!

gorenburg commented 3 years ago

@kentbrew confirming, all works now (not breaking user's behavior)! thanks for the update!

kentbrew commented 3 years ago

Thanks for checking in, and (everyone) thank you for the report, and for your patience while we took care of it.