Subtitles positioning breaks when container has relative position
If the div containing the video has position: relative, the Popcorn subtitles positioning will break. This is because the plugin is set up to position the subtitles container absolutely to the page, and with a position: relative ancestor it will instead be positioning it absolutely to that ancestor. So the subtitles appear in the wrong place.
I'm finishing up a patch that should fix this (by adding a position: relative wrapper) right now... while digging around I'm seeing some other subs cleanup that might be nice, but I'll leave that for later.
Comments and changes to this ticket
OK, made a patch that adds a wrapper element (extContainer) that is relatively positioned, then puts the subtitle container inside that and positions it absolutely on the wrapper. This also means that we don't have to worry about repositioning on video movement, although I left the updatePosition timer with a longer period because I'm not sure about a better way to deal with video resizing. I also removed now-obsolete tests that tested how the subtitles moved when the video was repositioned, because now the browser should handle that itself.
Note that I have only had a chance to test in a relatively narrow set of cases (including browsers etc.), so if other people could check and make sure this doesn't break in some browser or some use-case that would be good.
Pull request: https://github.com/mozilla/popcorn-js/pull/150
- State changed from new to peer-review-requested
Bump for this since the 1.3 rush is now over. I don't know that my solution is the most elegant, it might fail in some CSS situations that I haven't considered (but then, the current version fails in any relatively positioned element, so it may still be an improvement).
- State changed from peer-review-requested to feedback-requested
- Assigned user changed from Scott Downe to Charlie Stigler
- Milestone order changed from 125 to 0
If the video is moved without the container being moved. So, the video element itself, the subtitle won't go with it.
What if we just added our subtitle container to the body so absolute or relative, it'll mean the same thing. Then, in update position, we can get the video's bounding client rect, and check the left and top, which, I believe (correct me if I am wrong) are absolute to the body, no matter what. With this, we can update the subtitle's container.
- State changed from feedback-requested to peer-review-requested
- Assigned user changed from Charlie Stigler to Scott Downe
- Milestone order changed from 2 to 0
Good point, that's a much better system. I redid this to work that way, appending the subtitle container to body. As an extra upside of this, the old tests are still relevant with this change (and they pass).
Old pull request seems to have been closed for oldness, here's a new one:
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.