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

Commit

Permalink
Removes non-primitive types from UrlBar
Browse files Browse the repository at this point in the history 
Resolves #9756

Auditors: @bsclifton @bbondy

Test Plan:
  • Loading branch information
@NejcZdovc
NejcZdovc committed Jun 28, 2017
1 parent 22c07e6 commit d5beac9
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 86 deletions.
14 changes: 13 additions & 1 deletion app/common/lib/suggestion.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -708,5 +719,6 @@ module.exports = {
getAlexaSuggestions,
generateNewSuggestionsList,
generateNewSearchXHRResults,
filterSuggestionListByType
filterSuggestionListByType,
getNormalizedSuggestion
}
11 changes: 2 additions & 9 deletions app/renderer/components/navigation/navigationBar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -208,10 +204,7 @@ class NavigationBar extends React.Component {
: null
}
</div>
<UrlBar
titleMode={this.props.titleMode}
onStop={this.onStop}
/>
<UrlBar titleMode={this.props.titleMode} />
{
this.props.showPublisherToggle
? <div className='endButtons'>
Expand Down
127 changes: 53 additions & 74 deletions app/renderer/components/navigation/urlBar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -161,29 +153,28 @@ 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
case KeyCodes.DELETE:
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()
Expand All @@ -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 {
Expand All @@ -215,7 +206,7 @@ class UrlBar extends React.Component {
}
}

onClick (e) {
onClick () {
if (this.props.isSelected) {
windowActions.setUrlBarActive(true)
}
Expand All @@ -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
Expand Down Expand Up @@ -344,7 +319,7 @@ class UrlBar extends React.Component {
})
}

onFocus (e) {
onFocus () {
this.select()
windowActions.urlBarOnFocus(getCurrentWindowId())
}
Expand Down Expand Up @@ -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()
}
Expand All @@ -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()
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -563,10 +544,8 @@ class UrlBar extends React.Component {
</span>
}
{
this.shouldRenderUrlBarSuggestions
? <UrlBarSuggestions
menubarVisible={this.props.menubarVisible}
/>
this.props.showrUrlBarSuggestions
? <UrlBarSuggestions />
: null
}
</form>
Expand Down
5 changes: 3 additions & 2 deletions app/renderer/components/navigation/urlBarSuggestions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions test/unit/app/common/lib/suggestionTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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/')
})
})
})

0 comments on commit d5beac9

Please sign in to comment.