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.

Sign up for GitHub

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

Jump to bottom

Multiple fixes: old issue #548, current issues #754 and #902 #923

Merged
merged 6 commits into from
Jul 23, 2013
Merged

Multiple fixes: old issue #548, current issues #754 and #902 #923

merged 6 commits into from
Jul 23, 2013

Conversation

peterh-capella
Copy link
Contributor

In version 2.9, I reported issue #548, that controls hide when using keyboard to scrub even if the alwaysShowControls option is set to "true." Pull request #569 merged this fix, but did so in the /build directory. It appears that fix did not move forward with versions and the issue is occurring in the latest version. Adding fix in the /src directory so that future builds get the fix.

In browsing open issues, I found issue #754, which was also affecting me. Commenter marshallbu had a work-around that fixed the issue for me. I've added that fix to the /src and /build directories.

Added fix for issue #902 which hides the native captions that occur in Chrome in preference to the captions generated by mejs.

@peterh-capella
fix for controls that hide if keyboard scrubbing
80f92d3 
In version 2.9, I reported issue #548, that controls hide when using
keyboard to scrub even if the alwaysShowControls option is set to
"true." Pull request #569 merged this fix, but did so in the /build
directory. It appears that fix did not move forward with versions and
the issue is occurring in the latest version. Adding fix in the /src
directory so that future builds get the fix.
@peterh-capella
adding controls fix to build and minified versions
bf73229
@peterh-capella
Fix for issue #754, no duration display w/jQuery 1.9
784ad2f 
Removed leading and trailing spaces in timeAndDurationSeparator to work
around change in jQuery selector build rules as detailed in comment to
issue #754 by marshallbu.
@peterh-capella
timeAndDurationSeparator fix added to build
92e0476
@peterh-capella
Fix for issue #902
c753e35 
In build tracks function, I added a check to see if the browser will
attempt to do present captions natively. (Currently only a feature in
Chrome. Outside of mejs, my if statement would include a check to see
if any textTracks were declared (t.domNode.textTracks.length > 0), but
the buildtracks function already checks for player.tracks.length.) If
this is true, we prefer to use mejs captions and loop through the
textTracks to set their mode to "hidden" to prevent display.
@peterh-capella
textTracks fix added to /build
8e49158
@johndyer
Copy link
Collaborator

These are great. However, again, when changes are made to the build folder it makes it difficult to control what is a "release" and sometimes changes get lost when they are not also in the /src/ folder. I'll try to work these in, but in the future if you could only use the /src/ folder that would be very helpful.

@peterh-capella
Copy link
Contributor Author

I think I made these changes to both the /src/ and the /build/ directories. (At least that's what I intended.) I made the changes in /build/ for my own use as I assumed it would take some time for the updates to make it to the next release.

@johndyer
Copy link
Collaborator

Yes, I noticed you made them in both folders. My request is that you only commit changes to the /src/ folder so that changes to the /build/ folder will only reflect official releases. Of course, please do make custom builds as you need, but if you leave them uncommitted it makes it much easier to handle merges and make sure no one inadvertently downloads a development release.

But thanks again for your work. I appreciate it either way!

@peterh-capella
Copy link
Contributor Author

Gotcha! I was hoping there was a way to submit a pull request for only the /src/ directory, but didn't see how to do that. Glad the commits are useful in any case! Happy to help on your project, it's been a big help to me.

johndyer added a commit that referenced this pull request Jul 23, 2013
@johndyer
Merge pull request #923 from peterh-capella/master
e8a9e48 
Multiple fixes: old issue #548, current issues #754 and #902
@johndyer johndyer merged commit e8a9e48 into mediaelement:master Jul 23, 2013
phuongdh pushed a commit to avalonmediasystem/mediaelement that referenced this pull request Oct 18, 2013
@johndyer
Merge pull request mediaelement#923 from peterh-capella/master
8c8c2e7 
Multiple fixes: old issue mediaelement#548, current issues mediaelement#754 and mediaelement#902
rexblack pushed a commit to rexblack/mediaelement that referenced this pull request May 22, 2014
@johndyer
Merge pull request mediaelement#923 from peterh-capella/master
9631ebd 
Multiple fixes: old issue mediaelement#548, current issues mediaelement#754 and mediaelement#902
marmite22 pushed a commit to elucidat/mediaelement that referenced this pull request Dec 16, 2016
@johndyer
Merge pull request mediaelement#923 from peterh-capella/master
2371240 
Multiple fixes: old issue mediaelement#548, current issues mediaelement#754 and mediaelement#902
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@peterh-capella @johndyer