From 306649240a63715a681b70154caa62469b2227d8 Mon Sep 17 00:00:00 2001 From: bridiver Date: Sat, 9 Jan 2016 00:29:33 -0700 Subject: [PATCH] better handling of frames on window start --- docs/state.md | 1 - js/components/frame.js | 53 ++- js/components/main.js | 7 +- js/components/window.js | 15 +- js/state/frameStateUtil.js | 2 +- js/stores/appStore.js | 16 +- js/stores/windowStore.js | 27 +- package.json | 10 +- test/fixtures/click_with_target.html | 13 + test/fixtures/iframe_target_parent.html | 10 + test/fixtures/iframe_target_top.html | 9 + test/lib/brave.js | 45 ++ test/lib/server.js | 8 + test/navigationBarTest.js | 49 +- test/windowTest.js | 567 +++++++++++++++++++++--- 15 files changed, 714 insertions(+), 118 deletions(-) create mode 100644 test/fixtures/click_with_target.html create mode 100644 test/fixtures/iframe_target_parent.html create mode 100644 test/fixtures/iframe_target_top.html diff --git a/docs/state.md b/docs/state.md index 3d9d7e4f87d..7a4e1e93459 100644 --- a/docs/state.md +++ b/docs/state.md @@ -74,7 +74,6 @@ WindowStore activeMixedContent: boolean, // has active mixed content passiveMixedContent: boolean, // has passive mixed content }, - parentWindowKey: number, // the key of the window this frame was opened from parentFrameKey: number, // the key of the frame this frame was opened from contextMenuDetail: {...}, modalPromptDetail: {...}, diff --git a/js/components/frame.js b/js/components/frame.js index 35eedcfe9e1..f25a7191fdb 100644 --- a/js/components/frame.js +++ b/js/components/frame.js @@ -27,19 +27,22 @@ class Frame extends ImmutableComponent { } createWebview () { - while (this.webviewContainer.firstChild) { - this.webviewContainer.removeChild(this.webviewContainer.firstChild) - } // Create the webview dynamically because React doesn't whitelist all // of the attributes we need. - this.webview = document.createElement('webview') + this.webview = this.webview || document.createElement('webview') this.webview.setAttribute('preload', 'content/webviewPreload.js') if (this.props.frame.get('isPrivate')) { this.webview.setAttribute('partition', 'private-1') } + if (this.props.frame.get('guestInstanceId')) { + this.webview.setAttribute('data-guest-instance-id', this.props.frame.get('guestInstanceId')) + } this.webview.setAttribute('src', this.props.frame.get('src')) - this.webviewContainer.appendChild(this.webview) - this.addEventListeners() + + if (!this.webviewContainer.firstChild) { + this.webviewContainer.appendChild(this.webview) + this.addEventListeners() + } } componentDidMount () { @@ -51,8 +54,7 @@ class Frame extends ImmutableComponent { if (didSrcChange) { this.createWebview() } - if ((this.props.isActive && !prevProps.isActive && !this.props.frame.getIn(['navbar', 'urlbar', 'focused'])) || - (this.props.isActive && didSrcChange)) { + if (this.props.isActive && !prevProps.isActive) { this.webview.focus() } const activeShortcut = this.props.frame.get('activeShortcut') @@ -104,33 +106,35 @@ class Frame extends ImmutableComponent { } addEventListeners () { - // @see new-window event this.webview.addEventListener('focus', this.onFocus.bind(this)) - this.webview.addEventListener('new-window', (e) => { - // TODO handle _top, _parent and named frames - // also popup blocking and security restrictions!! + // @see new-window event + this.webview.addEventListener('new-window', (e, url, frameName, disposition, options) => { + e.preventDefault() + + let guestInstanceId = e.options.webPreferences && e.options.webPreferences.guestInstanceId + let windowOptions = e.options.windowOptions || {} + windowOptions.parentWindowKey = remote.getCurrentWindow().id + windowOptions.disposition = e.disposition - // @see dom open - // @see browsing context name or keyword - if (e.frameName.toLowerCase() === '_self') { - WindowActions.loadUrl(this.props.frame, e.url) - } else if (e.disposition === 'new-window' || e.frameName.toLowerCase() === '_blank') { + if (e.disposition === 'new-window' || e.disposition === 'new-popup') { AppActions.newWindow({ location: e.url, parentFrameKey: this.props.frame.get('key'), - parentWindowKey: remote.getCurrentWindow().id - }, e.options) + guestInstanceId + }, windowOptions) } else { WindowActions.newFrame({ location: e.url, parentFrameKey: this.props.frame.get('key'), - parentWindowKey: remote.getCurrentWindow().id, - openInForeground: e.disposition !== 'background-tab' + openInForeground: e.disposition !== 'background-tab', + guestInstanceId }) } }) + this.webview.addEventListener('destroyed', (e) => { + WindowActions.closeFrame(this.props.frames, this.props.frame) + }) this.webview.addEventListener('close', () => { - // security restrictions here? AppActions.closeWindow(remote.getCurrentWindow().id) }) this.webview.addEventListener('enter-html-full-screen', () => { @@ -162,6 +166,11 @@ class Frame extends ImmutableComponent { this.webview.canGoBack(), this.webview.canGoForward()) }) + this.webview.addEventListener('did-navigate', (e) => { + if (this.props.isActive && this.webview.getURL() !== Config.defaultUrl) { + this.webview.focus() + } + }) this.webview.addEventListener('did-start-loading', () => { WindowActions.onWebviewLoadStart( this.props.frame) diff --git a/js/components/main.js b/js/components/main.js index dfc516c1dae..e506aa8cb13 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -30,12 +30,6 @@ const FrameStateUtil = require('../state/frameStateUtil') class Main extends ImmutableComponent { componentDidMount () { - if (this.props.windowState.get('frames').isEmpty()) { - WindowActions.newFrame({ - location: Config.defaultUrl - }) - } - ipc.on(messages.STOP_LOAD, () => { electron.remote.getCurrentWebContents().send(messages.SHORTCUT_ACTIVE_FRAME_STOP) }) @@ -173,6 +167,7 @@ class Main extends ImmutableComponent { sortedFrames.map(frame => { - WindowActions.newFrame(frame) - }) + if (this.props.frames.length === 0) { + WindowActions.newFrame({ + location: Config.defaultUrl + }) + } else { + WindowStore.suspend() + this.props.frames.forEach(frame => { + WindowActions.newFrame(frame) + }) + WindowStore.resume() + } } render () { diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js index f465449046d..6be1437db93 100644 --- a/js/state/frameStateUtil.js +++ b/js/state/frameStateUtil.js @@ -135,7 +135,7 @@ export function addFrame (frames, frameOpts, newKey, activeFrameKey) { isPinned: frameOpts.isPinned, key: newKey, parentFrameKey: frameOpts.parentFrameKey, - parentWindowKey: frameOpts.parentWindowKey, + guestInstanceId: frameOpts.guestInstanceId, navbar: { searchSuggestions: true, focused: true, diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 12e89285734..3e92259d5ae 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -31,10 +31,8 @@ function navbarHeight () { return 75 } -const createWindow = (browserOpts, defaults, parentWindowKey) => { - browserOpts = browserOpts || {} - // clean up properties - delete browserOpts.webPreferences +const createWindow = (browserOpts, defaults) => { + let parentWindowKey = browserOpts.parentWindowKey browserOpts.width = firstDefinedValue(browserOpts.width, browserOpts.innerWidth, defaults.width) // height and innerHeight are the frame webview size @@ -156,6 +154,7 @@ function windowDefaults () { setDefaultWindowSize() return { + show: false, width: appState.get('defaultWindowWidth'), height: appState.get('defaultWindowHeight'), minWidth: 500, @@ -191,9 +190,10 @@ const handleAppAction = (action) => { appStore.emitChange() break case AppConstants.APP_NEW_WINDOW: - const frameOpts = action.frameOpts && action.frameOpts.toJS() || undefined - const browserOpts = action.browserOpts && action.browserOpts.toJS() || undefined - let mainWindow = createWindow(browserOpts, windowDefaults(), frameOpts && frameOpts.parentWindowKey) + const frameOpts = action.frameOpts && action.frameOpts.toJS() + const browserOpts = (action.browserOpts && action.browserOpts.toJS()) || {} + + let mainWindow = createWindow(browserOpts, windowDefaults()) if (action.restoredState) { mainWindow.webContents.once('dom-ready', () => { mainWindow.webContents.send('restore-state', action.restoredState) @@ -226,6 +226,8 @@ const handleAppAction = (action) => { mainWindow.loadURL('file://' + __dirname + '/../../app/index.html?' + queryString) } appStore.emitChange() + + mainWindow.show() break case AppConstants.APP_CLOSE_WINDOW: let appWindow = BrowserWindow.fromId(action.appWindowId) diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 2addd55894c..5d91f00d0c7 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -58,6 +58,7 @@ let currentKey = 0 const incrementNextKey = () => ++currentKey class WindowStore extends EventEmitter { + getState () { return windowState } @@ -67,7 +68,11 @@ class WindowStore extends EventEmitter { } emitChange () { - this.emit(CHANGE_EVENT) + if (!this.suspended) { + this.emit(CHANGE_EVENT) + } else { + this.emitOnResume = true + } } addChangeListener (callback) { @@ -77,6 +82,26 @@ class WindowStore extends EventEmitter { removeChangeListener (callback) { this.removeListener(CHANGE_EVENT, callback) } + + /** + * Temporarily suspend the emitting of change events for this store + * You can use this when you have multiple actions that can/should be accomplished in a single render + */ + suspend () { + this.suspended = true + } + + /** + * Resume the emitting of change events + * A change event will be emitted if any updates were made while the store was suspended + */ + resume () { + this.suspended = false + if (this.emitOnResume) { + this.emitOnResume = false + this.emitChange() + } + } } const windowStore = new WindowStore() diff --git a/package.json b/package.json index 290081c8f60..5e6b041f2c0 100644 --- a/package.json +++ b/package.json @@ -4,8 +4,8 @@ "description": "Brave laptop and desktop browser", "main": "./app/index.js", "scripts": { - "start": "node ./tools/start.js --debug=5858", - "start-brk": "node ./tools/start.js --debug-brk=5858", + "start": "node ./tools/start.js --debug=5858 -enable-logging --v=0 --enable-dcheck", + "start-brk": "node ./tools/start.js --debug-brk=5858 -enable-logging --v=0 --enable-dcheck", "watch-all": "npm run watch & npm run watch-test", "watch-test": "NODE_ENV=test webpack --watch", "watch": "webpack-dev-server", @@ -38,6 +38,10 @@ { "name": "Aubrey Keus", "email": "aubrey@brave.com" + }, + { + "name": "Brian Johnson", + "email": "bjohnson@brave.com" } ], "license": "MPL-2.0", @@ -89,7 +93,7 @@ "node-static": "^0.7.7", "node-uuid": "^1.4.7", "pre-commit": "^1.1.2", - "spectron": "^0.35.3", + "spectron": "^0.36.0", "standard": "^5.4.1", "style-loader": "^0.13.0", "webdriverio": "^3.3.0", diff --git a/test/fixtures/click_with_target.html b/test/fixtures/click_with_target.html new file mode 100644 index 00000000000..b8588abb793 --- /dev/null +++ b/test/fixtures/click_with_target.html @@ -0,0 +1,13 @@ + + + + click.with.target + + + none + name + name2 + self +