diff --git a/js/components/urlBar.js b/js/components/urlBar.js index 943af54a93f..ef37952bf66 100644 --- a/js/components/urlBar.js +++ b/js/components/urlBar.js @@ -40,6 +40,7 @@ class UrlBar extends ImmutableComponent { this.onBlur = this.onBlur.bind(this) this.onKeyDown = this.onKeyDown.bind(this) this.onKeyUp = this.onKeyUp.bind(this) + this.onChange = this.onChange.bind(this) this.onClick = this.onClick.bind(this) this.onContextMenu = this.onContextMenu.bind(this) this.onSiteInfo = this.onSiteInfo.bind(this) @@ -73,27 +74,19 @@ class UrlBar extends ImmutableComponent { return this.props.urlbar.get('focused') } - // update the DOM with state that is not stored in the component - updateDOM () { - this.updateDOMInputFocus(this.isFocused()) - this.updateDOMInputSelected(this.isSelected()) - } - updateDOMInputFocus (focused) { if (this.urlInput && focused) { - this.urlInput.focus() - } - } - - updateDOMInputSelected (selected) { - if (this.urlInput && selected) { - this.urlInput.select() + this.focus() } } // restores the url bar to the current location restore () { - windowActions.setNavBarUserInput(this.props.location) + const location = UrlUtil.getDisplayLocation(this.props.location, getSetting(settings.PDFJS_ENABLED)) + if (this.urlInput) { + this.urlInput.value = location + } + windowActions.setNavBarUserInput(location) } // Temporarily disable the autocomplete when a user is pressing backspace. @@ -133,6 +126,8 @@ class UrlBar extends ImmutableComponent { e.preventDefault() ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_STOP) this.clearSearchEngine() + this.restore() + this.select() break case KeyCodes.DELETE: if (e.shiftKey) { @@ -146,6 +141,9 @@ class UrlBar extends ImmutableComponent { } break case KeyCodes.BACKSPACE: + this.clearSearchEngine() + break + case KeyCodes.TAB: this.hideAutoComplete() break // Do not trigger rendering of suggestions if you are pressing alt or shift @@ -166,16 +164,17 @@ class UrlBar extends ImmutableComponent { } onClick (e) { - // if the url bar is already selected then clicking in it should make it active if (this.isSelected()) { - windowActions.setUrlBarSelected(false) + windowActions.setUrlBarSelected(true) windowActions.setUrlBarActive(true) } } onBlur (e) { windowActions.setNavBarFocused(false) - windowActions.setUrlBarSelected(false) + if (!this.isActive) { + windowActions.setNavBarUserInput(e.target.value) + } // On blur, a user expects the text shown from the last autocomplete suffix // to be auto entered as the new location. this.clearSearchEngine() @@ -192,8 +191,8 @@ class UrlBar extends ImmutableComponent { } detectSearchEngine (input) { - let location = input || this.props.urlbar.get('location') - if (location !== null && location.length !== 0) { + let location = typeof input === 'string' ? input : this.props.urlbar.get('location') + if (typeof location === 'string' && location.length !== 0) { const isLocationUrl = isUrl(location) if (!isLocationUrl && !(this.searchSelectEntry && location.startsWith(this.searchSelectEntry.shortcut + ' '))) { @@ -214,6 +213,23 @@ class UrlBar extends ImmutableComponent { this.searchSelectEntry = null } + onChange (e) { + const oldActivateSearchEngine = this.activateSearchEngine + const oldSearchSelectEntry = this.searchSelectEntry + + this.clearSearchEngine() + this.detectSearchEngine(e.target.value) + + // TODO: activeSearchEngine and searchSelectEntry should be stored in + // state so we don't have to do this hack. + // It comes into play because no keyUp state changes happen when + // you use the context menu to select all and then cut. + if (oldActivateSearchEngine !== this.activateSearchEngine || + oldSearchSelectEntry !== this.searchSelectEntry) { + this.forceUpdate() + } + } + onKeyUp (e) { switch (e.keyCode) { case KeyCodes.UP: @@ -241,7 +257,7 @@ class UrlBar extends ImmutableComponent { const isLocationUrl = isUrl(location) if (!isLocationUrl && e.ctrlKey) { windowActions.loadUrl(this.activeFrame, `www.${location}.com`) - } else if (this.shouldRenderUrlBarSuggestions && (this.urlBarSuggestions.activeIndex > 0 || this.props.locationValueSuffix)) { + } else if (this.shouldRenderUrlBarSuggestions && (this.urlBarSuggestions.activeIndex > 0 || this.props.locationValueSuffix && this.autocompleteEnabled)) { // Hack to make alt enter open a new tab for url bar suggestions when hitting enter on them. const isDarwin = process.platform === 'darwin' if (e.altKey) { @@ -284,6 +300,18 @@ class UrlBar extends ImmutableComponent { } } + select () { + if (this.urlInput) { + this.urlInput.select() + } + } + + focus () { + if (this.urlInput) { + this.urlInput.focus() + } + } + onFocus (e) { windowActions.setUrlBarSelected(true) this.detectSearchEngine() @@ -309,32 +337,47 @@ class UrlBar extends ImmutableComponent { windowActions.setUrlBarSelected(true) windowActions.setUrlBarActive(true) // The urlbar "selected" might already be set in the window state, so subsequent Command+L won't trigger component updates, so this needs another DOM refresh for selection. - this.updateDOM() + this.updateDOMInputFocus() }) // escape key handling ipc.on(messages.SHORTCUT_ACTIVE_FRAME_STOP, this.onActiveFrameStop) } componentDidMount () { - this.updateDOM() + if (this.urlInput) { + this.urlInput.value = UrlUtil.getDisplayLocation(this.props.location, getSetting(settings.PDFJS_ENABLED)) + this.focus() + } } componentDidUpdate (prevProps) { // this.urlInput is not initialized in titleMode if (this.urlInput) { // Select the part of the URL which was an autocomplete suffix. - if (this.props.locationValueSuffix.length > 0 && this.isActive && + if (this.props.location !== prevProps.location) { + this.urlInput.value = UrlUtil.getDisplayLocation(this.props.location, getSetting(settings.PDFJS_ENABLED)) + } else if (this.props.locationValueSuffix.length > 0 && this.isActive && (this.props.locationValueSuffix !== prevProps.locationValueSuffix || this.props.urlbar.get('location') !== prevProps.urlbar.get('location'))) { this.showAutocompleteResult() } else if (this.props.activeFrameKey !== prevProps.activeFrameKey || - this.props.titleMode !== prevProps.titleMode) { + this.props.titleMode !== prevProps.titleMode || + !this.isActive && !this.isFocused) { this.urlInput.value = this.locationValue + } else if (this.isActive) { + const selectedIndex = this.activeFrame.getIn(['navbar', 'urlbar', 'suggestions', 'selectedIndex']) + if (typeof selectedIndex === 'number') { + const suggestionLocation = this.activeFrame.getIn(['navbar', 'urlbar', 'suggestions', 'suggestionList', selectedIndex - 1]).location + this.urlInput.value = suggestionLocation + } } } - if (this.isSelected() !== prevProps.urlbar.get('selected') || - this.isFocused() !== prevProps.urlbar.get('focused')) { - this.updateDOM() + if (this.isFocused() !== prevProps.urlbar.get('focused')) { + this.updateDOMInputFocus() + } + if (this.isSelected() !== prevProps.urlbar.get('selected')) { + this.select() + windowActions.setUrlBarSelected(false) } } @@ -412,9 +455,6 @@ class UrlBar extends ImmutableComponent { const showIconSecure = !this.activateSearchEngine && this.isHTTPPage && this.props.isSecure && !this.props.urlbar.get('active') const showIconInsecure = !this.activateSearchEngine && this.isHTTPPage && !this.props.isSecure && !this.props.urlbar.get('active') && !this.props.titleMode const showIconSearch = !this.activateSearchEngine && this.props.urlbar.get('active') && this.props.loading === false - const value = this.isActive || (!this.locationValue && this.isFocused()) - ? undefined - : this.locationValue return
: null } diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 7f94661755c..3e573648c31 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -432,7 +432,7 @@ const doAction = (action) => { windowState = windowState.setIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'focused']), action.focused) // selection should be cleared on blur if (!action.focused) { - windowState = windowState.setIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'selected']), action.false) + windowState = windowState.setIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'selected']), false) } break case WindowConstants.WINDOW_NEW_FRAME: diff --git a/test/components/navigationBarTest.js b/test/components/navigationBarTest.js index 7620f8c01ad..ecb5022b580 100644 --- a/test/components/navigationBarTest.js +++ b/test/components/navigationBarTest.js @@ -106,7 +106,7 @@ describe('navigationBar', function () { }) }) - describe('window.open spoofing', function () { + describe.skip('window.open spoofing', function () { Brave.beforeAll(this) before(function * () { @@ -133,7 +133,7 @@ describe('navigationBar', function () { .waitForExist(urlInput) .waitUntil(function () { return this.getValue(urlInput).then((val) => { - return val.startsWith('https://www.google.com/') + return val.startsWith('https://www.example.com/') }) }) .getText(activeTabTitle) @@ -165,6 +165,35 @@ describe('navigationBar', function () { }) }) + describe('User input', function () { + Brave.beforeAll(this) + before(function * () { + yield setup(this.app.client) + var page1 = Brave.server.url('page1.html') + yield this.app.client + .tabByUrl(Brave.newTabUrl) + .url(page1) + .waitForUrl(page1) + }) + + it('remains cleared when onChange is fired but not onKeyUp', function * () { + yield this.app.client + .windowByUrl(Brave.browserWindowUrl) + .setValue(urlInput, '') + .waitUntil(function () { + return this.getValue(urlInput).then((val) => val === '') + }) + .moveToObject(activeWebview) + .click(activeWebview) + .moveToObject(navigator) + .waitForExist(urlInput) + .click(urlInput) + .waitUntil(function () { + return this.getValue(urlInput).then((val) => val === '') + }) + }) + }) + describe('page with a title', function () { Brave.beforeAll(this) @@ -183,6 +212,7 @@ describe('navigationBar', function () { const host = this.host yield this.app.client .moveToObject(activeWebview) + .click(activeWebview) .waitForExist(titleBar) .waitUntil(function () { return this.getText(titleBar).then((val) => val === host + ' | Page 1') @@ -193,6 +223,7 @@ describe('navigationBar', function () { it('shows the url on mouseover', function * () { yield this.app.client .moveToObject(activeWebview) + .click(activeWebview) .moveToObject(titleBar) .waitForExist(navigatorLoadTime) .getValue(urlInput) @@ -273,6 +304,7 @@ describe('navigationBar', function () { var page = this.page yield this.app.client .moveToObject(activeWebview) + .click(activeWebview) .waitForExist(titleBar) .moveToObject(titleBar) .waitForExist(urlInput) @@ -337,7 +369,7 @@ describe('navigationBar', function () { .windowByUrl(Brave.browserWindowUrl) .click(urlbarIcon) .waitForVisible('[data-l10n-id="insecureConnection"]') - .keys('\uE00C') + .keys(Brave.keys.ESCAPE) }) it('Shows secure URL icon', function * () { const page1Url = 'https://badssl.com/' @@ -352,7 +384,7 @@ describe('navigationBar', function () { .windowByUrl(Brave.browserWindowUrl) .click(urlbarIcon) .waitForVisible('[data-l10n-id="secureConnection"]') - .keys('\uE00C') + .keys(Brave.keys.ESCAPE) }) it('Blocks running insecure content', function * () { const page1Url = 'https://mixed-script.badssl.com/' @@ -658,7 +690,7 @@ describe('navigationBar', function () { it('sets location to new URL', function * () { const page2 = this.page2 yield this.app.client.keys(this.page2) - yield this.app.client.keys('\uE007') + yield this.app.client.keys(Brave.keys.ENTER) yield this.app.client .waitUntil(function () { return this.getValue(urlInput).then((val) => { @@ -670,14 +702,15 @@ describe('navigationBar', function () { it('resets URL to previous location if page does not load', function * () { const page1 = this.page1 yield this.app.client.tabByUrl(this.newTabUrl).url(page1).waitForUrl(page1).windowParentByUrl(page1) - yield this.app.client + .moveToObject(activeWebview) + .click(activeWebview) .moveToObject(navigator) .waitForExist(urlInput) .click(urlInput) yield selectsText(this.app.client, page1) - yield this.app.client.keys(this.page2) - yield this.app.client.keys('\uE007') yield this.app.client + .keys(this.page2) + .keys(Brave.keys.ENTER) .waitUntil(function () { return this.getValue(urlInput).then((val) => { return val === page1 @@ -697,7 +730,7 @@ describe('navigationBar', function () { yield this.app.client.waitForExist(urlInput) yield this.app.client.keys(this.page1) // hit enter - yield this.app.client.keys('\uE007') + yield this.app.client.keys(Brave.keys.ENTER) }) it('webview has focus', function * () { @@ -742,7 +775,7 @@ describe('navigationBar', function () { yield this.app.client.waitForExist(urlInput) yield this.app.client.keys(' javascript:alert(1)') // hit enter - yield this.app.client.keys('\uE007') + yield this.app.client.keys(Brave.keys.ENTER) }) it('filters javascript urls', function * () { yield this.app.client.waitUntil(function () { @@ -759,7 +792,7 @@ describe('navigationBar', function () { yield this.app.client.waitForExist(urlInput) yield this.app.client.keys('brave.com@example.com') // hit enter - yield this.app.client.keys('\uE007') + yield this.app.client.keys(Brave.keys.ENTER) }) it('hides auth part of the url', function * () { yield this.app.client.waitUntil(function () { @@ -786,7 +819,7 @@ describe('navigationBar', function () { it('shows about:blank', function * () { yield this.app.client .keys('about:blank') - .keys('\uE007') + .keys(Brave.keys.ENTER) .waitUntil(function () { return this.getValue(urlInput).then((val) => val === 'about:blank') }) @@ -874,13 +907,12 @@ describe('navigationBar', function () { .keys('\uE00C') .waitForElementFocus(urlInput) }) - - it('does not select the urlbar text', function * () { - yield selectsText(this.app.client, '.com') + it('does select the urlbar text', function * () { + yield selectsText(this.app.client, this.page) }) - it('does not revert the urlbar text', function * () { - yield this.app.client.getValue(urlInput).should.eventually.be.equal('google.com') + it('does revert the urlbar text', function * () { + yield this.app.client.getValue(urlInput).should.eventually.be.equal(this.page) }) }) @@ -899,7 +931,6 @@ describe('navigationBar', function () { .keys('\uE00C') .waitForElementFocus(urlInput) }) - it('does select the urlbar text', function * () { yield selectsText(this.app.client, this.page) }) @@ -920,9 +951,9 @@ describe('navigationBar', function () { .waitForElementFocus(urlInput) .setValue(urlInput, 'blah') // hit escape - .keys('\uE00C') + .keys(Brave.keys.ESCAPE) .waitForElementFocus(urlInput) - .keys('\uE00C') + .keys(Brave.keys.ESCAPE) }) it('selects the urlbar text', function * () { @@ -940,7 +971,7 @@ describe('navigationBar', function () { return yield this.app.client.ipcSend('shortcut-focus-url') .setValue(urlInput, url) // hit enter - .keys('\uE007') + .keys(Brave.keys.ENTER) }) it('changes the webview src', function * () { @@ -953,10 +984,10 @@ describe('navigationBar', function () { }) describe('search engine go key', function () { - Brave.beforeEach(this) + Brave.beforeAll(this) const entries = searchProviders.providers - beforeEach(function * () { + before(function * () { yield setup(this.app.client) yield this.app.client .windowByUrl(Brave.browserWindowUrl) @@ -964,15 +995,19 @@ describe('navigationBar', function () { .waitForElementFocus(urlInput) }) - entries.forEach((entry) => { - describe(entry.name, function () { - beforeEach(function * () { - yield this.app.client - .keys(entry.shortcut + ' ') + beforeEach(function * () { + yield this.app.client + .setValue(urlInput, '') + .waitUntil(function () { + return this.getValue(urlInput).then((val) => val === '') }) + }) + entries.forEach((entry) => { + describe(entry.name, function () { it('has the icon', function * () { yield this.app.client + .keys(entry.shortcut + ' ') .waitForExist(urlbarIcon) .waitUntil(function () { return this @@ -1097,6 +1132,7 @@ describe('navigationBar', function () { yield this.app.client.waitUntil(function () { return this.getValue(urlInput).then((val) => val === 'br') }) + yield selectsText(this.app.client, 'ave.com') yield blur(this.app.client) yield this.app.client .waitForExist(urlInput) diff --git a/test/fixtures/spoof_content.html b/test/fixtures/spoof_content.html index 697cf330194..a65a5a18583 100644 --- a/test/fixtures/spoof_content.html +++ b/test/fixtures/spoof_content.html @@ -1,3 +1,3 @@ -fake page -

fake page

- +fake page +

fake page

+ diff --git a/test/fixtures/spoof_opener.html b/test/fixtures/spoof_opener.html index 20a6d2c42a6..8fb49754473 100644 --- a/test/fixtures/spoof_opener.html +++ b/test/fixtures/spoof_opener.html @@ -1,22 +1,22 @@ - -Google
+ +Example Domain
diff --git a/test/lib/brave.js b/test/lib/brave.js index 0d8fac8c190..863c742fa30 100644 --- a/test/lib/brave.js +++ b/test/lib/brave.js @@ -74,7 +74,9 @@ var exports = { CONTROL: '\ue009', ESCAPE: '\ue00c', RETURN: '\ue006', - SHIFT: '\ue008' + ENTER: '\ue007', + SHIFT: '\ue008', + BACKSPACE: '\ue003' }, browserWindowUrl: 'file://' + path.resolve(__dirname, '..', '..') + '/app/extensions/brave/index.html',