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

URL bar autocomplete mouseover does not interfere with typed URL (fixes #3012) #3128

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions js/actions/windowActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -489,8 +489,8 @@ const windowActions = {
setUrlBarSuggestions: function (suggestionList, selectedIndex) {
dispatch({
actionType: WindowConstants.WINDOW_SET_URL_BAR_SUGGESTIONS,
suggestionList,
selectedIndex
suggestionList: suggestionList,
selectedIndex: selectedIndex
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change doesn't seem necessary, any reason for it?

})
},

Expand Down
26 changes: 14 additions & 12 deletions js/components/urlBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ var searchProviders = require('../data/searchProviders')
const searchIconSize = 16
const UrlUtil = require('../lib/urlutil')

const EventUtil = require('../lib/eventUtil')
const eventElHasAncestorWithClasses = EventUtil.eventElHasAncestorWithClasses

const { isUrl, isIntermediateAboutPage } = require('../lib/appUrlUtil')

class UrlBar extends ImmutableComponent {
Expand Down Expand Up @@ -203,8 +206,11 @@ class UrlBar extends ImmutableComponent {
windowActions.setUrlBarSelected(false)
// On blur, a user expects the text shown from the last autocomplete suffix
// to be auto entered as the new location.
this.updateLocationToSuggestion()
this.clearSearchEngine()

if (!eventElHasAncestorWithClasses(e, ['urlBarSuggestions', 'urlbarForm'])) {
this.updateLocationToSuggestion()
}
}

updateLocationToSuggestion () {
Expand Down Expand Up @@ -250,9 +256,13 @@ class UrlBar extends ImmutableComponent {
}

onActiveFrameStop () {
this.restore()
windowActions.setUrlBarSelected(true)
windowActions.setUrlBarActive(false)
if (this.isFocused()) {
windowActions.setUrlBarActive(false)
if (!this.shouldRenderUrlBarSuggestions) {
this.restore()
windowActions.setUrlBarSelected(true)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's some kind of a rebase issue here, please rebase from current master

}
}

componentWillMount () {
Expand Down Expand Up @@ -300,14 +310,6 @@ class UrlBar extends ImmutableComponent {
}

get locationValue () {
// If there's a selected autocomplete entry, we just want to show its location
if (this.props.suggestionIndex) {
const suggestionLocation = this.props.urlbar.getIn(['suggestions', 'suggestionList', this.props.suggestionIndex - 1]).location
if (suggestionLocation) {
return suggestionLocation
}
}

let location = this.props.urlbar.get('location')
const history = this.props.history
if (isIntermediateAboutPage(location) && history.size > 0) {
Expand Down
3 changes: 2 additions & 1 deletion js/components/urlBarSuggestions.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class UrlBarSuggestions extends ImmutableComponent {
}
items = items.concat(suggestions.map((suggestion, i) => {
const currentIndex = index + i
const selected = this.activeIndex === currentIndex + 1 || currentIndex === 0 && this.props.locationValueSuffix
const selected = this.activeIndex === currentIndex + 1 || (!this.activeIndex && currentIndex === 0 && this.props.locationValueSuffix)
return <li data-index={currentIndex + 1}
onMouseOver={this.onMouseOver.bind(this)}
onClick={suggestion.onClick}
Expand Down Expand Up @@ -205,6 +205,7 @@ class UrlBarSuggestions extends ImmutableComponent {
} else {
windowActions.loadUrl(this.activeFrame, location)
windowActions.setUrlBarActive(false)
windowActions.setUrlBarPreview(null)
this.blur()
}
}
Expand Down
23 changes: 22 additions & 1 deletion js/lib/eventUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,28 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */

const isDarwin = process.platform === 'darwin'
module.exports.isForSecondaryAction = (e) =>
module.exports.isForSecondaryAction = (e) => {
e.ctrlKey && !isDarwin ||
e.metaKey && isDarwin ||
e.button === 1
}

module.exports.eventElHasAncestorWithClasses = (e, classesToCheck) => {
let node = e.target

while (node) {
let classMatch = classesToCheck.map(
function (className) {
return (node.classList ? node.classList.contains(className) : false)
}
).includes(true)

if (classMatch) {
return true
}

node = node.parentNode
}

return false
}
15 changes: 12 additions & 3 deletions js/stores/windowStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,20 @@ const updateNavBarInput = (loc, frameStatePath = activeFrameStatePath()) => {
* @param suggestionList - The suggestion list to use to figure out the suffix.
*/
const updateUrlSuffix = (suggestionList) => {
const suggestion = suggestionList && suggestionList.get(0)
let selectedIndex = windowState.getIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'suggestions', 'selectedIndex']))

if (!selectedIndex) {
selectedIndex = 0
} else {
selectedIndex--
}

const suggestion = suggestionList && suggestionList.get(selectedIndex)
let suffix = ''
if (suggestion) {
const selectedIndex = windowState.getIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'suggestions', 'selectedIndex']))
const autocompleteEnabled = windowState.getIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'suggestions', 'autocompleteEnabled']))
if (!selectedIndex && autocompleteEnabled) {

if (autocompleteEnabled) {
const location = windowState.getIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'location']))
const index = suggestion.location.toLowerCase().indexOf(location.toLowerCase())
if (index !== -1) {
Expand Down Expand Up @@ -467,6 +475,7 @@ const doAction = (action) => {
break
case WindowConstants.WINDOW_SET_URL_BAR_SUGGESTIONS:
windowState = windowState.setIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'suggestions', 'selectedIndex']), action.selectedIndex)

if (action.suggestionList !== undefined) {
windowState = windowState.setIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'suggestions', 'suggestionList']), action.suggestionList)
}
Expand Down