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

[text tracks] take into account the video element parent container s… #2291

Conversation

jeffcunat
Copy link
Contributor

@jeffcunat jeffcunat commented Nov 14, 2017

Fix issue #2241
This PR takes into account the fact that the parent container of the video element can have a different size than the video element itself.
It also takes into account the relative positioning of the video element inside its parent (ex: vertically centered video element in fullscreen mode)

@Gontran-Molotov : could you check if it is OK for your use cases ?

@epiclabsDASH epiclabsDASH added this to the v2.6.4 milestone Nov 15, 2017
}

function getVideoRelativeOffsetLeft() {
return element && element.parentNode ? element.getBoundingClientRect().x - element.parentNode.getBoundingClientRect().x : NaN;
Copy link
Contributor

Choose a reason for hiding this comment

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

@jeffcunat,Looking at MDN (https://developer.mozilla.org/en-US/docs/Web/API/Element/getBoundingClientRect) seems object returned by getBoundClientRect() doesn't include x/y properties in Edge/IE/Safari.

@jeffcunat
Copy link
Contributor Author

jeffcunat commented Nov 17, 2017

@epiclabsDASH your remark is good, and I will use element.getBoundingClientRect().top and .left that is more widely used. I have also some other improvements ongoing

…nd manage changes of the video element position relatively to its parent (during resize)
@epiclabsDASH epiclabsDASH merged commit 62bb689 into Dash-Industry-Forum:development Nov 17, 2017
@bbert bbert deleted the SubtitlesPositionWithBiggerParent branch September 19, 2018 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants