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.

Sign up for GitHub

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

fix critical js error in some case #26

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix critical js error in some case #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aad9f72
fix poster for Dailymotion
Bouki May 30, 2016
a475573
avoid fatal js error if player is not ready
Bouki May 30, 2016
0778454
sync with es5
Bouki May 30, 2016
e0f3e41
added class vjs-dailymotion to the video container
Bouki May 30, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions es5/videojs-dailymotion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ var Dailymotion = (function (_Tech) {
url: options.source.src
};

setTimeout(function() {
this.el_.parentNode.className += ' vjs-dailymotion';
}.bind(this));

// If we are not on a server, don't specify the origin (it will crash)
if (window.location.protocol !== 'file:') {
this.params.origin = window.location.protocol + '//' + window.location.hostname;
Expand All @@ -66,7 +70,7 @@ var Dailymotion = (function (_Tech) {

if (typeof this.videoId !== 'undefined') {
this.setTimeout(function () {
_this.setPoster('//api.dailymotion.com/video/' + _this.videoId + '?fields=poster_url&ads=false');
_this.setPoster('https://api.dailymotion.com/video/' + _this.videoId + '?fields=thumbnail_large_url');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'll be better to stick to //api. rather than enforcing https://, that way it'll ensure compatibility from all sort of requests. Don't you think so?

}, 100);
}

Expand Down Expand Up @@ -118,6 +122,16 @@ var Dailymotion = (function (_Tech) {
}, {
key: 'loadApi',
value: function loadApi() {

var baseObjectLoader = this;

if (document.getElementById(this.options_.techId) == null) {
setTimeout(function() {
baseObjectLoader.loadApi();
}, 50);
return null;
}

this.dmPlayer = new DM.player(this.options_.techId, {
video: this.videoId,
width: this.options_.width,
Expand Down Expand Up @@ -213,8 +227,14 @@ var Dailymotion = (function (_Tech) {
}, {
key: 'setPoster',
value: function setPoster(poster) {
this.poster_ = poster;
this.trigger('posterchange');

var baseClass = this;
$.getJSON(poster, function(data) {
// Set the low resolution first
baseClass.poster_ = data.thumbnail_large_url;
baseClass.trigger('posterchange');
});

}

/**
Expand Down
26 changes: 22 additions & 4 deletions src/videojs-dailymotion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ class Dailymotion extends Tech {
url: options.source.src
};

setTimeout(function() {
this.el_.parentNode.className += ' vjs-dailymotion';
}.bind(this));

// If we are not on a server, don't specify the origin (it will crash)
if (window.location.protocol !== 'file:') {
this.params.origin = window.location.protocol + '//' + window.location.hostname;
Expand All @@ -41,8 +45,8 @@ class Dailymotion extends Tech {
this.videoId = this.parseSrc(options.source.src);

if (typeof this.videoId !== 'undefined') {
this.setTimeout(() => {
this.setPoster('//api.dailymotion.com/video/' + this.videoId + '?fields=poster_url&ads=false');
this.setTimeout(function () {
_this.setPoster('https://api.dailymotion.com/video/' + _this.videoId + '?fields=thumbnail_large_url');
}, 100);
}

Expand Down Expand Up @@ -93,6 +97,16 @@ class Dailymotion extends Tech {
}

loadApi() {

var baseObjectLoader = this;

if (document.getElementById(this.options_.techId) == null) {
setTimeout(function() {
baseObjectLoader.loadApi();
}, 50);
return null;
}

this.dmPlayer = new DM.player(this.options_.techId, {
video: this.videoId,
width: this.options_.width,
Expand Down Expand Up @@ -182,8 +196,12 @@ class Dailymotion extends Tech {
}

setPoster(poster) {
this.poster_ = poster;
this.trigger('posterchange');
var baseClass = this;
$.getJSON(poster, function(data) {
Copy link
Owner

@benjipott benjipott Jul 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Videojs get instead if jquey
videojs.xhr({

  •   body: someJSONString,

  •   uri: "/foo",

  •   headers: {

  •     "Content-Type": "application/json"

  •   }

  • }, function (err, resp, body) {

  •   // check resp.statusCode

  • });

https://github.com/videojs/video.js/blob/b7fdf21a05fca4158041b95b30f3950495414444/src/js/video.js#L516

// Set the low resolution first
baseClass.poster_ = data.thumbnail_large_url;
baseClass.trigger('posterchange');
});
}

/**
Expand Down