#728 ✓ staged
Scott Downe

Normalization of Vimeo into Baseplayer

Reported by Scott Downe | September 20th, 2011 @ 05:27 PM | in 1.0 Release (closed)

In #423, I decided to do youtube separate from vimeo because of the sheer size of it all.

This ticket it to update vimeo to use the baseplayer for defaults.

Comments and changes to this ticket

  • Scott Downe

    Scott Downe September 22nd, 2011 @ 03:12 PM

    • Assigned user set to “Scott Downe”
  • cadecairos

    cadecairos September 27th, 2011 @ 04:32 PM

    • State changed from “open” to “assigned”
    • Assigned user changed from “Scott Downe” to “cadecairos”
  • cadecairos

    cadecairos September 30th, 2011 @ 12:11 AM

    • State changed from “assigned” to “peer-review-requested”
    • Assigned user changed from “cadecairos” to “David Seifried”
  • David Seifried

    David Seifried October 6th, 2011 @ 11:23 AM

    • State changed from “peer-review-requested” to “review-needs-work”
    • Assigned user changed from “David Seifried” to “cadecairos”

    Fails lint over 1 semi-colon on line 197.

    Inside your volumeUpdate function on line 96 you assigning two variables values but never declare them ( as far as I can tell ). Also, is m even really necessary to declare at all?

    Your mute function on 113 is empty

    Also your load listener for the vimeo container doesnt do anything other than log the following:

    console.log("asdfasdf");
    

    I dont know if this was for testing or if there is functionality that needs to run on load.

    Lines 136 to 168 could probably use a bit of whitespace padding as well, just to make it a bit easier to read.

    In popcorn.vimeo.html

    Some single quotes need to be switched over to double quotes.
    Some comments are on the end of the line as well, probably dont event need these as they just say seconds

    In popcorn.vimeo.unit.js

    Line 1 - extra whitespace needed after first (
    line 180 - end of line comment
    line 204, 250 - needs some whitespace padding inside two sets of ( )
    line 234 - needs a extra whitespace before last )
    line 242 - needs extra whitespace after first (
    line 273 - has an odd comment, dont know if its necessary
    line 290 - needs some whitespace padding around the 0
    line 314 - whitespace padding before last )
    line 341 - 340, 351 - change single quotes to double quotes

    Also I seem to be getting an error in the console when im running the demo:

    Unsafe JavaScript attempt to access frame with URL http://localhost/~dseif/popcorn-js/players/vimeo/popcorn.vimeo.html from frame with URL http://webmademovies.org/. Domains, protocols and ports must match.

  • cadecairos

    cadecairos October 11th, 2011 @ 11:42 AM

    • State changed from “review-needs-work” to “peer-review-requested”
    • Assigned user changed from “cadecairos” to “David Seifried”
  • David Seifried

    David Seifried October 12th, 2011 @ 10:37 AM

    • State changed from “peer-review-requested” to “review-needs-work”
    • Assigned user changed from “David Seifried” to “cadecairos”

    In popcorn.vimeo.html:

    Lines 7, 8, 16, 20 need some whitespace after the ":"
    The whole files spacing is off, this is going to need to be fixed. It seems to be using 4 spaces as indentation instead of 2.
    Rogue console.log on 159
    End of line comments need to either be moved up the the line above or removed all togethor ( the seconds ones can probably be removed )
    Take a look through the whole file for spacing after colons as there are some more

    In popcorn.vimeo.js:

    Line 181 has a check for < 0 and > 1

    Other than this everything looks good to go.

    Still passes lint, unit tests pass in FF 7, Chrome, Opera, Safari.

  • cadecairos

    cadecairos October 17th, 2011 @ 11:00 AM

    • State changed from “review-needs-work” to “peer-review-requested”
    • Assigned user changed from “cadecairos” to “David Seifried”

    https://github.com/cadecairos/popcorn-js/tree/t728

    fixed all style issues in popcorn.vimeo.html

    "Line 181 has a check for < 0 and > 1"

    This is correct, as the setter function only accepts values between 0 and 1, so if anything outside that range is passed in, it will simply return the current volume of the Vimeo object.

  • David Seifried

    David Seifried October 17th, 2011 @ 11:57 AM

    • State changed from “peer-review-requested” to “super-review-requested”
    • Assigned user changed from “David Seifried” to “Scott Downe”

    Everything looks awesome now man. Passes lint. Ran vimeo unit tests in the following browsers and got full passses:

    • FF 7
    • FF 3.6
    • Most recent Chrome
    • Opera 11.*
    • Safari most recent

    Styling looks good now.

    PR+

  • Scott Downe

    Scott Downe October 20th, 2011 @ 02:02 PM

    • State changed from “super-review-requested” to “review-needs-work”
    • Assigned user changed from “Scott Downe” to “cadecairos”

    You're going to need to do what I had to do in #781 and #782. These are pretty high priority.

    Lower priority:

    The example page says:

    "Volume (0-100): 1" but should be "volume (0-1): 1" with one being max.

    Finally, the "toggle Volume" button doesn't seem to do anything. Either make it do something or remove it.

    Other than that, working fine. Tests pass.

  • cadecairos

    cadecairos October 24th, 2011 @ 10:47 AM

    • State changed from “review-needs-work” to “super-review-requested”
    • Assigned user changed from “cadecairos” to “Scott Downe”
    • Milestone order changed from “45” to “0”

    All done. Volume now returns 0-1 values, div size defaults if not specified, html page updated, toggle volume button working.

    https://github.com/cadecairos/popcorn-js/tree/t728

  • Scott Downe

    Scott Downe October 25th, 2011 @ 10:59 AM

    • Assigned user changed from “Scott Downe” to “cadecairos”
    • State changed from “super-review-requested” to “review-looks-good”

    I think we're all done here. Good work!

  • cadecairos

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Popcorn.js is an HTML5 video framework that lets you bring elements of the web into your videos.

Popcorn.js is a project of Web Made Movies, Mozilla's Open Video Lab.

Shared Ticket Bins

Referenced by

Pages