Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
give webview focus on fullscreen fixes #3464
Browse files Browse the repository at this point in the history
don't show findbar in webview fullscreen
exit webview fullscreen on tab change
auditors: @bbondy
  • Loading branch information
bridiver committed Aug 26, 2016
1 parent 850c2f2 commit 58fbf81
Showing 1 changed file with 20 additions and 5 deletions.
25 changes: 20 additions & 5 deletions js/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const clipboard = global.require('electron').clipboard
const FullScreenWarning = require('./fullScreenWarning')
const debounce = require('../lib/debounce.js')
const getSetting = require('../settings').getSetting
const config = require('../constants/config')
const settings = require('../constants/settings')
const FindBar = require('./findbar.js')
const { aboutUrls, isSourceAboutUrl, isTargetAboutUrl, getTargetAboutUrl, getBaseUrl, isNavigatableAboutPage } = require('../lib/appUrlUtil')
Expand Down Expand Up @@ -324,6 +325,19 @@ class Frame extends ImmutableComponent {
}
}

enterHtmlFullScreen () {
if (this.webview) {
this.webview.executeScriptInTab(config.braveExtensionId, 'document.documentElement.webkitRequestFullScreen()', {})
this.webview.focus()
}
}

exitHtmlFullScreen () {
if (this.webview) {
this.webview.executeScriptInTab(config.braveExtensionId, 'document.webkitExitFullscreen()', {})
}
}

componentDidUpdate (prevProps, prevState) {
const cb = () => {
if (this.webRTCPolicy !== this.getWebRTCPolicy()) {
Expand All @@ -337,11 +351,12 @@ class Frame extends ImmutableComponent {
}
// make sure the webview content updates to
// match the fullscreen state of the frame
if (prevProps.isFullScreen !== this.props.isFullScreen) {
if (this.props.isFullScreen) {
this.webview.executeJavaScript('document.webkitRequestFullscreen()')
if (prevProps.isFullScreen !== this.props.isFullScreen ||
(this.props.isFullScreen && !this.props.isActive)) {
if (this.props.isFullScreen && this.props.isActive) {
this.enterHtmlFullScreen()
} else {
this.webview.executeJavaScript('document.webkitExitFullscreen()')
this.exitHtmlFullScreen()
}
}
this.webview.setAudioMuted(this.props.audioMuted || false)
Expand Down Expand Up @@ -1048,7 +1063,7 @@ class Frame extends ImmutableComponent {
: null
}
{
this.props.findbarShown
this.props.findbarShown && !this.props.isFullScreen

This comment has been minimized.

Copy link
@bbondy

bbondy Aug 27, 2016

Member

I think it's the case that someone can work in full screen on a page and want to find in page no?

This comment has been minimized.

Copy link
@bridiver

bridiver Aug 27, 2016

Author Collaborator

maybe for browser fullscreen, but html fullscreen?

This comment has been minimized.

Copy link
@bridiver

bridiver Aug 27, 2016

Author Collaborator

just fyi - the reason I disabled this is because it caused focus issues for the fullscreen webview and prevented escape from working properly

This comment has been minimized.

Copy link
@bbondy

bbondy Aug 27, 2016

Member

even for html fullscreen, although if it's a video element then it's not needed. But some people work fully in full screen view so this will cause bug reports.

This comment has been minimized.

Copy link
@bbondy

bbondy Aug 27, 2016

Member

since this is only for html full screen and not browser window full screen it's ok for now, but we might get bug reports later for non video full screen requests, but I think it's rare so this is ok.

? <FindBar
onFind={this.onFind}
onFindHide={this.onFindHide}
Expand Down

0 comments on commit 58fbf81

Please sign in to comment.