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

Commit

Permalink
Remove isFocused call from hide autofill event
Browse files Browse the repository at this point in the history
This gets called pretty frequently by chromium into electron even if it's not showing

Auditors: @bridiver
  • Loading branch information
bbondy committed Jan 24, 2017
1 parent 658dc43 commit 63edc2e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 6 deletions.
2 changes: 1 addition & 1 deletion js/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ class Frame extends ImmutableComponent {
contextMenus.onShowAutofillMenu(e.suggestions, e.rect, this.frame)
})
this.webview.addEventListener('hide-autofill-popup', (e) => {
if (this.webview.isFocused()) {
if (this.props.contextMenuDetail && this.props.contextMenuDetail.get('type') === 'autofill') {

This comment has been minimized.

Copy link
@bridiver

bridiver Jan 24, 2017

Collaborator

I actually don't think this will work correctly. I did something very similar in windowState and we had to revert to using isFocused. Check with @darkdh

This comment has been minimized.

Copy link
@bbondy

bbondy Jan 25, 2017

Author Member

lmk if it's a problem @darkdh from testing here it works but I could definitely be missing some edge case.

This comment has been minimized.

Copy link
@darkdh

darkdh Jan 25, 2017

Member

@bridiver is right. We have to check whether webview is focused or the autofill entry won't be selected by clicking.

This comment has been minimized.

Copy link
@darkdh

darkdh Jan 25, 2017

Member

fixed in fbb8a01

windowActions.autofillPopupHidden(this.props.tabId)
}
})
Expand Down
12 changes: 7 additions & 5 deletions js/components/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,8 @@ class Main extends ImmutableComponent {
const customTitlebar = this.customTitlebar
const versionInformation = this.props.appState.getIn(['about', 'brave', 'versionInformation'])
const braveryDefaults = Immutable.fromJS(siteSettings.braveryDefaults(this.props.appState, appConfig))
const shouldAllowWindowDrag = !this.props.windowState.get('contextMenuDetail') &&
const contextMenuDetail = this.props.windowState.get('contextMenuDetail')
const shouldAllowWindowDrag = !contextMenuDetail &&
!this.props.windowState.get('bookmarkDetail') &&
!siteInfoIsVisible &&
!braveryPanelIsVisible &&
Expand All @@ -914,10 +915,10 @@ class Main extends ImmutableComponent {
onMouseDown={this.onMouseDown}
onClick={this.onClickWindow}>
{
this.props.windowState.get('contextMenuDetail')
contextMenuDetail
? <ContextMenu
lastZoomPercentage={activeFrame && activeFrame.get('lastZoomPercentage')}
contextMenuDetail={this.props.windowState.get('contextMenuDetail')}
contextMenuDetail={contextMenuDetail}
selectedIndex={customTitlebar.contextMenuSelectedIndex} />
: null
}
Expand All @@ -939,7 +940,7 @@ class Main extends ImmutableComponent {
<Menubar
template={customTitlebar.menubarTemplate}
selectedIndex={customTitlebar.menubarSelectedIndex}
contextMenuDetail={this.props.windowState.get('contextMenuDetail')}
contextMenuDetail={contextMenuDetail}
autohide={getSetting(settings.AUTO_HIDE_MENU)}
lastFocusedSelector={customTitlebar.lastFocusedSelector} />
<WindowCaptionButtons windowMaximized={customTitlebar.isMaximized} />
Expand Down Expand Up @@ -1146,7 +1147,7 @@ class Main extends ImmutableComponent {
shouldAllowWindowDrag={shouldAllowWindowDrag && !isWindows}
activeFrameKey={activeFrame && activeFrame.get('key') || undefined}
windowWidth={window.innerWidth}
contextMenuDetail={this.props.windowState.get('contextMenuDetail')}
contextMenuDetail={contextMenuDetail}
sites={appStateSites}
selectedFolderId={this.props.windowState.getIn(['ui', 'bookmarksToolbar', 'selectedFolderId'])} />
: null
Expand Down Expand Up @@ -1207,6 +1208,7 @@ class Main extends ImmutableComponent {
prefOpenInForeground={getSetting(settings.SWITCH_TO_NEW_TABS)}
onCloseFrame={this.onCloseFrame}
frameKey={frame.get('key')}
contextMenuDetail={contextMenuDetail}
partition={frameStateUtil.getPartition(frame)}
key={frame.get('key')}
settings={['about:preferences', 'about:history', 'about:adblock'].includes(getBaseUrl(frame.get('location')))
Expand Down

1 comment on commit 63edc2e

@bbondy
Copy link
Member Author

@bbondy bbondy commented on 63edc2e Jan 25, 2017

Choose a reason for hiding this comment

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

Please sign in to comment.