From d5beac9b67d71a90baa2cb3099eaf268fa8893d5 Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Wed, 28 Jun 2017 18:26:43 +0200 Subject: [PATCH] Removes non-primitive types from UrlBar Resolves #9756 Auditors: @bsclifton @bbondy Test Plan: --- app/common/lib/suggestion.js | 14 +- .../components/navigation/navigationBar.js | 11 +- app/renderer/components/navigation/urlBar.js | 127 ++++++++---------- .../navigation/urlBarSuggestions.js | 5 +- test/unit/app/common/lib/suggestionTest.js | 26 ++++ 5 files changed, 97 insertions(+), 86 deletions(-) diff --git a/app/common/lib/suggestion.js b/app/common/lib/suggestion.js index 4af51274cef..aac6c5c91e5 100644 --- a/app/common/lib/suggestion.js +++ b/app/common/lib/suggestion.js @@ -687,6 +687,17 @@ const filterSuggestionListByType = (suggestionList) => { } } +const getNormalizedSuggestion = (suggestionList, activeIndex) => { + let suggestion = '' + let normalizedSuggestion = '' + if (suggestionList && suggestionList.size > 0) { + suggestion = suggestionList.getIn([activeIndex || 0, 'location'], '') + normalizedSuggestion = normalizeLocation(suggestion) + } + + return normalizedSuggestion +} + module.exports = { sortingPriority, sortByAccessCountWithAgeDecay, @@ -708,5 +719,6 @@ module.exports = { getAlexaSuggestions, generateNewSuggestionsList, generateNewSearchXHRResults, - filterSuggestionListByType + filterSuggestionListByType, + getNormalizedSuggestion } diff --git a/app/renderer/components/navigation/navigationBar.js b/app/renderer/components/navigation/navigationBar.js index be11a8c01ef..a21a61b53da 100644 --- a/app/renderer/components/navigation/navigationBar.js +++ b/app/renderer/components/navigation/navigationBar.js @@ -86,11 +86,7 @@ class NavigationBar extends React.Component { if (this.props.navbar.getIn(['urlbar', 'focused'])) { windowActions.setUrlBarActive(false) const shouldRenderSuggestions = this.props.navbar.getIn(['urlbar', 'suggestions', 'shouldRender']) === true - const suggestionList = this.props.navbar.getIn(['urlbar', 'suggestions', 'suggestionList']) - if (!shouldRenderSuggestions || - // TODO: Once we take out suggestion generation from within URLBarSuggestions we can remove this check - // and put it in shouldRenderUrlBarSuggestions where it belongs. See https://github.com/brave/browser-laptop/issues/3151 - !suggestionList || suggestionList.size === 0) { + if (!shouldRenderSuggestions) { windowActions.setUrlBarSelected(true) } } @@ -208,10 +204,7 @@ class NavigationBar extends React.Component { : null } - + { this.props.showPublisherToggle ?
diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index 410eb318963..100e7e745fd 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -26,7 +26,6 @@ const frameStateUtil = require('../../../../js/state/frameStateUtil') const siteSettings = require('../../../../js/state/siteSettings') const tabState = require('../../../common/state/tabState') const siteSettingsState = require('../../../common/state/siteSettingsState') -const menuBarState = require('../../../common/state/menuBarState') // Utils const cx = require('../../../../js/lib/classSet') @@ -36,7 +35,8 @@ const contextMenus = require('../../../../js/contextMenus') const {eventElHasAncestorWithClasses, isForSecondaryAction} = require('../../../../js/lib/eventUtil') const {getBaseUrl, isUrl} = require('../../../../js/lib/appUrlUtil') const {getCurrentWindowId} = require('../../currentWindow') -const {normalizeLocation} = require('../../../common/lib/suggestion') +const {normalizeLocation, getNormalizedSuggestion} = require('../../../common/lib/suggestion') +const {isDarwin} = require('../../../common/lib/platformUtil') const publisherUtil = require('../../../common/lib/publisherUtil') // Icons @@ -99,13 +99,6 @@ class UrlBar extends React.Component { windowActions.setRenderUrlBarSuggestions(false) } - get activeIndex () { - if (this.props.suggestionList === null) { - return -1 - } - return this.props.selectedIndex - } - onKeyDown (e) { if (!this.props.isActive) { windowActions.setUrlBarActive(true) @@ -125,11 +118,10 @@ class UrlBar extends React.Component { const isLocationUrl = isUrl(location) if (!isLocationUrl && e.ctrlKey) { appActions.loadURLRequested(this.props.activeTabId, `www.${location}.com`) - } else if (this.shouldRenderUrlBarSuggestions && - ((typeof this.activeIndex === 'number' && this.activeIndex >= 0) || + } else if (this.props.showrUrlBarSuggestions && + ((typeof this.props.activeIndex === 'number' && this.props.activeIndex >= 0) || (this.props.urlbarLocationSuffix && this.props.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) { if (isDarwin) { e.metaKey = true @@ -161,20 +153,20 @@ class UrlBar extends React.Component { windowActions.setRenderUrlBarSuggestions(false) break case KeyCodes.UP: - if (this.shouldRenderUrlBarSuggestions) { + if (this.props.showrUrlBarSuggestions) { windowActions.previousUrlBarSuggestionSelected() e.preventDefault() } break case KeyCodes.DOWN: - if (this.shouldRenderUrlBarSuggestions) { + if (this.props.showrUrlBarSuggestions) { windowActions.nextUrlBarSuggestionSelected() e.preventDefault() } break case KeyCodes.ESC: e.preventDefault() - this.props.onStop() + this.onStop() this.restore() this.select() break @@ -182,8 +174,7 @@ class UrlBar extends React.Component { if (e.shiftKey) { const selectedIndex = this.props.urlbarLocationSuffix.length > 0 ? 1 : this.props.selectedIndex if (selectedIndex !== undefined) { - const suggestionLocation = this.props.suggestion.location - appActions.removeSite({ location: suggestionLocation }) + appActions.removeSite({ location: this.props.suggestionLocation }) } } else { this.hideAutoComplete() @@ -193,7 +184,7 @@ class UrlBar extends React.Component { this.hideAutoComplete() break case KeyCodes.TAB: - if (this.shouldRenderUrlBarSuggestions) { + if (this.props.showrUrlBarSuggestions) { if (e.shiftKey) { windowActions.previousUrlBarSuggestionSelected() } else { @@ -215,7 +206,7 @@ class UrlBar extends React.Component { } } - onClick (e) { + onClick () { if (this.props.isSelected) { windowActions.setUrlBarActive(true) } @@ -225,26 +216,10 @@ class UrlBar extends React.Component { windowActions.urlBarOnBlur(getCurrentWindowId(), e.target.value, this.props.urlbarLocation, eventElHasAncestorWithClasses(e, ['urlBarSuggestions', 'urlbarForm'])) } - get suggestionLocation () { - const selectedIndex = this.props.selectedIndex - if (typeof selectedIndex === 'number') { - const suggestion = this.props.suggestion - if (suggestion) { - return suggestion.location - } - } - } - updateAutocomplete (newValue) { - let suggestion = '' - let suggestionNormalized = '' - if (this.props.suggestionList && this.props.suggestionList.size > 0) { - suggestion = this.props.suggestionList.getIn([this.activeIndex || 0, 'location']) || '' - suggestionNormalized = normalizeLocation(suggestion) - } const newValueNormalized = normalizeLocation(newValue) - if (suggestionNormalized.startsWith(newValueNormalized) && suggestionNormalized.length > 0) { - const newSuffix = suggestionNormalized.substring(newValueNormalized.length) + if (this.props.normalizedSuggestion.startsWith(newValueNormalized) && this.props.normalizedSuggestion.length > 0) { + const newSuffix = this.props.normalizedSuggestion.substring(newValueNormalized.length) this.setValue(newValue, newSuffix) this.urlInput.setSelectionRange(newValue.length, newValue.length + newSuffix.length + 1) return true @@ -344,7 +319,7 @@ class UrlBar extends React.Component { }) } - onFocus (e) { + onFocus () { this.select() windowActions.urlBarOnFocus(getCurrentWindowId()) } @@ -415,11 +390,6 @@ class UrlBar extends React.Component { return '' } - get shouldRenderUrlBarSuggestions () { - return this.props.shouldRender === true && - this.props.suggestionList && this.props.suggestionList.size > 0 - } - onNoScript () { windowActions.setNoScriptVisible() } @@ -428,6 +398,16 @@ class UrlBar extends React.Component { contextMenus.onUrlBarContextMenu(e) } + onStop () { + ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_STOP) + if (this.props.isFocused) { + windowActions.setUrlBarActive(false) + if (!this.props.shouldRenderSuggestion) { + windowActions.setUrlBarSelected(true) + } + } + } + mergeProps (state, ownProps) { const currentWindow = state.get('currentWindow') const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map() @@ -459,44 +439,46 @@ class UrlBar extends React.Component { searchShortcut = new RegExp('^' + provider.get('shortcut') + ' ', 'g') searchURL = provider.get('search') } + const suggestionList = urlbar.getIn(['suggestions', 'suggestionList']) + const scriptsBlocked = activeFrame.getIn(['noScript', 'blocked']) + const enableNoScript = siteSettingsState.isNoScriptEnabled(state, braverySettings) + const activeIndex = suggestionList === null ? -1 : selectedIndex const props = {} - - props.activeTabId = activeTabId - props.activeFrameKey = activeFrame.get('key') - props.frameLocation = frameLocation - props.displayURL = displayURL + // used in renderer + props.isWideURLbarEnabled = getSetting(settings.WIDE_URL_BAR) + props.publisherButtonVisible = publisherUtil.shouldShowAddPublisherButton(state, location, publisherId) + props.titleMode = ownProps.titleMode props.hostValue = hostValue + props.urlbarLocation = urlbarLocation props.title = activeFrame.get('title', '') - props.scriptsBlocked = activeFrame.getIn(['noScript', 'blocked']) - props.enableNoScript = siteSettingsState.isNoScriptEnabled(state, braverySettings) - props.showNoScriptInfo = props.enableNoScript && props.scriptsBlocked && props.scriptsBlocked.size - props.hasSuggestionMatch = urlbar.getIn(['suggestions', 'hasSuggestionMatch']) + props.displayURL = displayURL props.startLoadTime = activeFrame.get('startLoadTime') props.endLoadTime = activeFrame.get('endLoadTime') props.loading = activeFrame.get('loading') - props.noScriptIsVisible = currentWindow.getIn(['ui', 'noScriptInfo', 'isVisible']) || false - props.menubarVisible = ownProps.menubarVisible - props.publisherButtonVisible = publisherUtil.shouldShowAddPublisherButton(state, location, publisherId) - props.onStop = ownProps.onStop - props.titleMode = ownProps.titleMode - props.urlbarLocation = urlbarLocation - props.urlbarLocationSuffix = urlbar.getIn(['suggestions', 'urlSuffix']) - props.selectedIndex = selectedIndex - props.suggestionList = urlbar.getIn(['suggestions', 'suggestionList']) - props.suggestion = urlbar.getIn(['suggestions', 'suggestionList', selectedIndex - 1]) - props.shouldRender = urlbar.getIn(['suggestions', 'shouldRender']) - props.urlbarLocation = urlbarLocation + props.showDisplayTime = !props.titleMode && props.displayURL === location + props.showNoScriptInfo = enableNoScript && scriptsBlocked && scriptsBlocked.size props.isActive = urlbar.get('active') - props.isSelected = urlbar.get('selected') + props.showrUrlBarSuggestions = urlbar.getIn(['suggestions', 'shouldRender']) === true && + suggestionList && suggestionList.size > 0 + + // used in other functions + props.activeFrameKey = activeFrame.get('key') + props.urlbarLocation = urlbarLocation props.isFocused = urlbar.get('focused') - props.isWideURLbarEnabled = getSetting(settings.WIDE_URL_BAR) - props.searchSelectEntry = urlbarSearchDetail + props.frameLocation = frameLocation + props.isSelected = urlbar.get('selected') + props.noScriptIsVisible = currentWindow.getIn(['ui', 'noScriptInfo', 'isVisible'], false) + props.selectedIndex = selectedIndex + props.suggestionLocation = urlbar.getIn(['suggestions', 'suggestionList', selectedIndex - 1, 'location']) + props.normalizedSuggestion = getNormalizedSuggestion(suggestionList, activeIndex) + props.activeTabId = activeTabId + props.urlbarLocationSuffix = urlbar.getIn(['suggestions', 'urlSuffix']) props.autocompleteEnabled = urlbar.getIn(['suggestions', 'autocompleteEnabled']) props.searchURL = searchURL props.searchShortcut = searchShortcut - props.showDisplayTime = !props.titleMode && props.displayURL === location - props.menubarVisible = menuBarState.isMenuBarVisible(currentWindow) + props.shouldRenderSuggestion = urlbar.getIn(['suggestions', 'shouldRender']) === true + props.activeIndex = activeIndex return props } @@ -535,7 +517,6 @@ class UrlBar extends React.Component { onContextMenu={this.onContextMenu} data-l10n-id='urlbar' className={cx({ - private: this.private, testHookLoadDone: !this.props.loading })} id='urlInput' @@ -563,10 +544,8 @@ class UrlBar extends React.Component { } { - this.shouldRenderUrlBarSuggestions - ? + this.props.showrUrlBarSuggestions + ? : null } diff --git a/app/renderer/components/navigation/urlBarSuggestions.js b/app/renderer/components/navigation/urlBarSuggestions.js index a4df5204164..41355d4e4bd 100644 --- a/app/renderer/components/navigation/urlBarSuggestions.js +++ b/app/renderer/components/navigation/urlBarSuggestions.js @@ -18,6 +18,7 @@ const locale = require('../../../../js/l10n') const suggestions = require('../../../common/lib/suggestion') const frameStateUtil = require('../../../../js/state/frameStateUtil') const domUtil = require('../../lib/domUtil') +const menuBarState = require('../../../common/state/menuBarState') const {getCurrentWindowId} = require('../../currentWindow') class UrlBarSuggestions extends React.Component { @@ -75,14 +76,14 @@ class UrlBarSuggestions extends React.Component { const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map() const urlBar = activeFrame.getIn(['navbar', 'urlbar'], Immutable.Map()) const documentHeight = domUtil.getStyleConstants('navbar-height') - const menuHeight = ownProps.menubarVisible ? 30 : 0 + const menubarVisible = menuBarState.isMenuBarVisible(currentWindow) + const menuHeight = menubarVisible ? 30 : 0 const props = {} // used in renderer props.maxHeight = document.documentElement.offsetHeight - documentHeight - 2 - menuHeight // used in functions - props.menubarVisible = ownProps.menubarVisible props.suggestionList = urlBar.getIn(['suggestions', 'suggestionList']) // TODO (nejc) improve, use primitives props.hasSuggestionMatch = urlBar.getIn(['suggestions', 'hasSuggestionMatch']) props.activeIndex = props.suggestionList === null diff --git a/test/unit/app/common/lib/suggestionTest.js b/test/unit/app/common/lib/suggestionTest.js index 60b94c4222b..ea05b1cd546 100644 --- a/test/unit/app/common/lib/suggestionTest.js +++ b/test/unit/app/common/lib/suggestionTest.js @@ -448,4 +448,30 @@ describe('suggestion unit tests', function () { }) }) }) + + describe('getNormalizedSuggestion', function () { + const suggestionList = Immutable.fromJS([ + { + location: 'https://www.brave.com/' + }, + { + location: 'https://clifton.io/' + } + ]) + + it('suggestion list is empty', function () { + const result = suggestion.getNormalizedSuggestion(Immutable.List()) + assert.equal(result, '') + }) + + it('active index is not provided', function () { + const result = suggestion.getNormalizedSuggestion(suggestionList) + assert.equal(result, 'brave.com/') + }) + + it('everything is provided', function () { + const result = suggestion.getNormalizedSuggestion(suggestionList, 1) + assert.equal(result, 'clifton.io/') + }) + }) })