#381 ✓ staged
danman

Subtitles don't move with video

Reported by danman | March 2nd, 2011 @ 08:01 PM | in 0.6

http://matrix.senecac.on.ca/~dsventura/popcorn-js/plugins/subtitle/...

I was making unit tests for the subtitle plugin when I stumbled across this bug. When the tests are finished, the information above the video pushes the video down, but not the subtitles. This is due to the subtitle div having an absolute position.
Also, the subtitle font does not change depending on the video size. If the video is made smaller, should the subtitles be smaller as well or stay large so they're easy to read?

I'll start hacking on this but any insight is appreciated.
Here's some source: https://github.com/DanVentura/popcorn-js/tree/bug127/plugins/subtitle

Comments and changes to this ticket

  • db48x

    db48x March 2nd, 2011 @ 08:32 PM

    • Assigned user set to “Scott Downe”

    Scott, this would be fixed if we went with my suggested css from #378 :)

  • Scott Downe

    Scott Downe March 8th, 2011 @ 04:50 PM

    • State changed from “open” to “assigned”
    • Milestone set to 0.5
    • Milestone order changed from “41” to “0”

    I am not convinced we can properly position subtitles in all cases without absolute positioning, but I don't mind being proven wrong.

    I had problems making the subtitle div a child to the video element, despite the fact I think that is a bad place to put things standards wise, it would of solved enough problems to make it worth it, it would not display in all browsers. It is possible my code became fragmented while testing and debugging.

    I also know from experience we cannot assume the video has a proper parent, and that the video is placed in the top left of said parent.

    db is correct, my solution to #378 does not fix this, and in order to make that work without adding the subtitle to the video as a child you would have to attach an event listener to the video element for size or position changing, and if so, update the subtitle's position. Which is very plausible.

    I will put this into 0.5 for now, and begin working on it shortly. If I am done in time, I will try to sneak it into 0.4

  • Scott Downe

    Scott Downe March 8th, 2011 @ 04:52 PM

    If we want the subtitle div to scale with the video, that will need to be an event listener for sure, and that is also plausible.

  • Scott Downe

    Scott Downe April 9th, 2011 @ 01:43 PM

    • Milestone changed from 0.5 to 0.6
    • Milestone order changed from “21” to “0”
  • annasob

    annasob April 15th, 2011 @ 07:20 PM

    Scott is this resolved by #467

  • Scott Downe

    Scott Downe May 12th, 2011 @ 07:12 PM

    • State changed from “assigned” to “peer-review-requested”
    • Assigned user cleared.
    • Milestone order changed from “9” to “0”

    https://github.com/ScottDowne/popcorn-js/commits/t381

    Note, I did NOT do subtitle scaling in this ticket. It is much more complex than I expected regarding aspect ratio and centering. So I am breaking that down into smaller tickets.

    Tests added.

  • Rick

    Rick May 13th, 2011 @ 04:48 PM

    • State changed from “peer-review-requested” to “super-review-requested”
    • Assigned user set to “annasob”

    Tested subtitle test suite, full pass in:

    • FF 3.6, 4
    • Chrome 11, 12
  • annasob

    annasob May 14th, 2011 @ 04:33 AM

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

    I ran these tests on Mac OSX an I am getting failures this is a brand new machine so I am running on latest Safari, Chrome, and new Firefox Nightly build. I am getting the following errors:
    - subtitle top position default, expected: - Chrome: expected: expected: "239px" result: "247px", diff: "239px" "247px" - Safari: expected: "239px" result: "247px", diff: "239px" "247px" - FF nightly: expected: "244px" result: "247px", diff: "244px" "247px"

    • subtitle top position moved, expected:
      • Chrome: expected: expected: "657px" result: "678px", diff: "657px" "678px"
      • Safari: expected: "657px" result: "682px", diff: "657px" "682px"
      • FF nightly: expected: "657px" result: "682px", diff: "657px" "682px"

    Can you take a look Scott maybe ask dseif to run them

  • Scott Downe

    Scott Downe May 16th, 2011 @ 08:20 PM

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

    https://github.com/ScottDowne/popcorn-js/commits/t381

    I had to completely rework the way I was testing because browsers were returning slightly different pixel positions.

    I capture the old position's pixel to test against, check that the new position is NOT the same as the old. This is to ensure the video's position HAS changed, and that the subttiles changed with it.

    Now that I know the positions have changed, it is safe to check the subtitle position against the video.

    If I just did the second, my tests would pass if the video's position didn't change at all.

    Commented this funked up method as best as I could in the tests. Basically, I am guessing, but I guess form both ends, so if one guess is wrong, the other will be wrong in the opposite direction, thus ensuring ONE method will fail the test.

  • Rick

    Rick May 16th, 2011 @ 08:35 PM

    This looks magnificent, test is slick too... very clean.

    Tested

    • FF 3.6, 4.x

    • Chrome 11, 12, 13

  • annasob

    annasob May 17th, 2011 @ 11:26 AM

    • State changed from “super-review-requested” to “review-looks-good”

    this looks good tests now pass on Fedora Chrome Firefox 3.6 and 4.0
    - subtitle demo runs - lint passes

    SR+

  • annasob

    annasob May 17th, 2011 @ 11:27 AM

    • State changed from “review-looks-good” to “staged”

    Staged in annasob/popcorn-js commit

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