-
Notifications
You must be signed in to change notification settings - Fork 235
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 player integration code removed from the dailymotion js sdk #983
Conversation
if not @element or @element.nodeType != Node.ELEMENT_NODE | ||
throw new Error("Invalid player element in DailymotionPlayer(), requires an existing HTML element: " + @element) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Under what conditions could this happen? If this could realistically happen to a user it'd be better to add a warning banner on the page rather than throw an error from the script in which case the user probably won't realize what happened.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just the validation code from dailymotion’s deprecated DM.player function. I don't think it's a realistic end user situation, it made more sense as a warning to outside programmers of the dailymotion js sdk that they'd passed in a bad parameter. In sync, it’s only useful for warning about a potential future programming error, and can safely be removed if you’d rather.
This is precisely why I threw my hands up in the air. This is the same reason I haven't added Rumble support. Their APIs seem geared to allowing publishers to customize embeds on their own sites and the idea of an unrelated 3rd party using their embeds is persona non grata. I appreciate your efforts to subvert their enshittifications. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, left this in Pending
if not @element or @element.nodeType != Node.ELEMENT_NODE | ||
throw new Error("Invalid player element in DailymotionPlayer(), requires an existing HTML element: " + @element) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just the validation code from dailymotion’s deprecated DM.player function. I don't think it's a realistic end user situation, it made more sense as a warning to outside programmers of the dailymotion js sdk that they'd passed in a bad parameter. In sync, it’s only useful for warning about a potential future programming error, and can safely be removed if you’d rather.
Closes #980 |
The broken dailymotion player described in #980 was caused by Dailymotion removing direct player creation support from their js SDK. They’d set up a method to remove it a while ago, which they turned on with their commit 75b4102. The js SDK, documented here, is loaded from https://api.dmcdn.net/all.js by sync & other projects. The SDK’s github readme currently says:
The new integration method they point to there seems to demand making a dailymotion account, configuring a custom player w/ a unique ID, and loading their scripts based on that ID . This is a can of worms, and it's not clear how this could work for sync in general.
Fortunately, it’s still possible to create a Dailymotion player with their js SDK’s object interface, instead of the functional interface that vanished when they decided to “Sunset player API v2". This PR does so to get Dailymotion sync support working again.