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

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

Merged
merged 1 commit into from
Nov 21, 2014

Conversation

mattfawcett
Copy link
Contributor

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.

Currenly works in IE11 (the latest) using the ms prefix.
@Afterster Afterster mentioned this pull request Oct 28, 2014
@thepag
Copy link
Contributor

thepag commented Nov 20, 2014

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
Copy link
Contributor Author

@thepag - Just signed the CLA.

@thepag
Copy link
Contributor

thepag commented Nov 20, 2014

Cheers @mattfawcett

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

@mattfawcett
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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

@thepag
Copy link
Contributor

thepag commented Nov 20, 2014

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

@thepag
Copy link
Contributor

thepag commented Nov 20, 2014

@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
Copy link
Contributor Author

@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
Copy link
Contributor

thepag commented Nov 20, 2014

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
Copy link
Contributor

thepag commented Nov 21, 2014

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 thepag merged commit 16e8c38 into jplayer:master Nov 21, 2014
thepag added a commit that referenced this pull request Nov 21, 2014
@thepag
Copy link
Contributor

thepag commented Nov 21, 2014

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants