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

better inline-videos parsing #148

Closed
Sudrien opened this issue Oct 31, 2015 · 4 comments
Closed

better inline-videos parsing #148

Sudrien opened this issue Oct 31, 2015 · 4 comments

Comments

@Sudrien
Copy link
Contributor

Sudrien commented Oct 31, 2015

#147 suggested adding youtube.be support, there is room for further improvements.

  • Force embed to https - testing shows youtube will do this anyways, might as well take away one opportunity to spoof urls.
  • Support t= - or, embeds can be started at a certain time during the video

Caveats:

  • t=#h##m##s is not the format used in embeds - rather, it's start=### - a single integer, in seconds ( see https://developers.google.com/youtube/player_parameters#start ). At least t= uses a consistent order, so a single regex to break them out should be enough.
  • Looking at multiple url parameters or unknown order means we really should be doing proper url & parameter parsing - something complex enough to be a part of Candy.Util, or use an external library.
@Sudrien
Copy link
Contributor Author

Sudrien commented Nov 21, 2015

Want to get down results of experimenting, no push request code yet.

args.message = jQuery.parseHTML(args.message).map(function(a) {
  if(a.outerHTML){ //dom element of any sort
    switch(a.hostname) { // a link with an href
      case "youtube.com":
      case "www.youtube.com":
        return a.search; //do iframe replacement here, v= important, t= needs conversion if exist
        break;
      case "youtu.be":
        return a.pathname + a.search; //do iframe replacement here, pathname important, t= needs conversion if exist
        break;
      default:  //undefined - not a link
        return a.outerHTML;
      }
    }
  else { //this is a string that has to be wrapped in somthing
    return a.textContent;
    }
  }).join("");

@Sudrien
Copy link
Contributor Author

Sudrien commented Dec 2, 2015

Pushed first attempt to branch. I'm wondering if I should be displaying the original URL or a sanitized one - leaning towards sanitized, but not very hard. @mweibel - is this code looking good to you?

(note to self: remove debug console calls)

@Sudrien
Copy link
Contributor Author

Sudrien commented Dec 4, 2015

Pushed second attempt, in response to @benlangfeld 's comments.
Also added Vimeo support - it's a site I find myself linked to frequently enough.

@benlangfeld
Copy link
Member

Go ahead and open a PR, mark it WIP and hold discussion there @Sudrien. It's easier to review and comment then :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants