search

jplayer / jPlayer

jPlayer : HTML5 Audio & Video for jQuery
http://jplayer.org/
Other
4.6k stars 1.47k forks source link

Add support for native fullscreen api in Internet explorer. #213

Closed mattfawcett closed 9 years ago

mattfawcett commented 10 years ago

IE11 has native fullscreen support which this change adds support for. This allows you to get a full size player when embedding in an iframe, which the existing fullWindow did not have the ability to do.

thepag commented 9 years ago

Hello @mattfawcett would you please sign our CLA

While merging this fix, I will also investigate the key bindings not working in Webkit/blink based browser.

mattfawcett commented 9 years ago

@thepag - Just signed the CLA.

thepag commented 9 years ago

Cheers @mattfawcett

I'm not sure why CLAhub is reporting that not all contributors have signed this PR.

mattfawcett commented 9 years ago

I wonder if it might be because the email address in my commit messages does not match that of the CLA? Although I would have just expected it to work based on Github usernames.

Afterster commented 9 years ago

Problem is that your commit email is not linked to a github account. GitHub cannot link the commit to a github account, then CLAHub cannot link it neither.

mattfawcett commented 9 years ago

Just added my other email to github so hopefully it will work now.

thepag commented 9 years ago

I need to be able to poke CLAHub to refresh your details.

thepag commented 9 years ago

@mattfawcett have you checked that your git client (or CLI git) settings are using the correct address?

Failing that, make a test PR that I would later delete/close, but that new one should work. If the new PR fails, then you must have something messed up in your setting somewhere.

mattfawcett commented 9 years ago

@thepag Yeah, my git cli is configured to use my work email address, and I can see that is the email address that is the author in the commit for this pull request. But up until earlier today, that email address was not referenced on my Github account. So it looks like either CLAHub need to repull my account details from Github, or maybe they don't even support multiple email addresses per client.

thepag commented 9 years ago

I just checked again now and it's working @mattfawcett

After merging this PR, I will fix the WebKit Full Screen and Keyboard input bug.

thepag commented 9 years ago

Testing this now in IE11 Win 8.1 with IE in desktop mode: Clicking the fullscreen button works, but if I use the key bindings f key then it only goes into the (fallback) full window mode.

I will look around a bit for confirmation, but I'd appreciate any help. This is kinda like the almost opposite of the next issue I'm going to solve. The key bindings not working in full screen mode on wekbit/blink.

thepag commented 9 years ago

Well ok, the webkit family really do not get on with each other.

elem.webkitRequestFullScreen(Element.ALLOW_KEYBOARD_INPUT);

The Element.ALLOW_KEYBOARD_INPUT is defined in all the webkit browsers (Chrome, Safari & Opera) but Safari will completely ignore the request if you send the flag parameter. Fair enough, ignore it, but go full screen!

So either:

  1. we have a horrible UA sniffer in there
  2. we accept the security feature and accept key bindings do not work in Webkit
  3. we ignore the minority (desktop) Safari browser and it will not even go full screen.

The latter is not acceptable. I have decided to go with no change and Chrome/opera keybindings will not work in full screen. Safari keys will not work either in full screen, but there is nothing we can do about that.

I see that mozilla propose another Flag to request fullscreen with keyboard support, so that the user can receive a special prompt warning them. But that is over 2 years old and does not seem to have made any progress.

The bias is towards ARIA controls over keybindings anyway.

TLDR; No change - webkit keys in fullscreen will continue to not work.