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

Commit

Permalink
Navigating to new page should reset find state, but keep last text
Browse files Browse the repository at this point in the history
Fix #5286

Auditors: @diracdeltas
  • Loading branch information
bbondy committed Oct 31, 2016
1 parent 7f0208f commit 170a14e
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 2 deletions.
1 change: 1 addition & 0 deletions app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ module.exports.cleanPerWindowData = (perWindowData, isShutdown) => {
if (frame.findDetail) {
delete frame.findDetail.numberOfMatches
delete frame.findDetail.activeMatchOrdinal
delete frame.findDetail.internalFindStatePresent
}
delete frame.findbarShown
// Don't restore full screen state
Expand Down
2 changes: 1 addition & 1 deletion js/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,7 @@ class Frame extends ImmutableComponent {

this.webview.addEventListener('did-navigate', (e) => {
if (this.props.findbarShown) {
windowActions.setFindbarShown(this.frame, false)
this.props.onFindHide()
}

for (let message in this.notificationCallbacks) {
Expand Down
5 changes: 4 additions & 1 deletion js/components/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,9 @@ class Main extends ImmutableComponent {
windowActions.setFindbarShown(activeFrame, false)
webviewActions.stopFindInPage()
windowActions.setFindDetail(activeFrame, Immutable.fromJS({
internalFindStatePresent: false
internalFindStatePresent: false,
numberOfMatches: -1,
activeMatchOrdinal: 0
}))
}

Expand Down Expand Up @@ -1203,6 +1205,7 @@ class Main extends ImmutableComponent {
ledgerInfo={this.props.appState.get('ledgerInfo') || new Immutable.Map()}
publisherInfo={this.props.appState.get('publisherInfo') || new Immutable.Map()}
frameSiteSettings={this.frameSiteSettings(frame.get('location'))}
onFindHide={this.onFindHide}
enableNoScript={this.enableNoScript(this.frameSiteSettings(frame.get('location')))}
isPreview={frame.get('key') === this.props.windowState.get('previewFrameKey')}
isActive={FrameStateUtil.isFrameKeyActive(this.props.windowState, frame.get('key'))}
Expand Down
33 changes: 33 additions & 0 deletions test/components/frameTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,39 @@ describe('findbar', function () {
assert.equal(match, '2 of 2')
})

it('findbar input remembered but no active ordinals after navigation until RETURN key', function * () {
const url2 = Brave.server.url('find_in_page2.html')
yield this.app.client
.showFindbar()
.waitForElementFocus(findBarInput)
.setValue(findBarInput, 'Brad')
.waitUntil(function () {
return this.getValue(findBarInput).then((val) => val === 'Brad')
})
let match = yield this.app.client.getText(findBarMatches)
assert.equal(match, '0 matches')

yield this.app.client
.waitForVisible(findBarMatches)
.tabByUrl(Brave.newTabUrl)
.url(url2)
.waitForUrl(url2)
.windowParentByUrl(url2)
.waitUntil(function () {
return this.getAttribute('webview[data-frame-key="1"]', 'src').then((src) => src === url2)
})
// No findbar
.waitForVisible(findBarInput, 500, true)
.showFindbar()
.waitForElementFocus(findBarInput)
// Matches shouldn't be shown until enter is pressed
.waitForVisible(findBarMatches, 500, true)
.keys(Brave.keys.RETURN)
.waitForVisible(findBarMatches)
match = yield this.app.client.getText(findBarMatches)
assert.equal(match, '1 of 1')
})

it('remembers findbar input when switching frames', function * () {
const url = Brave.server.url('find_in_page.html')
yield this.app.client
Expand Down
9 changes: 9 additions & 0 deletions test/fixtures/find_in_page2.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<html>
<head>
<meta charset="utf-8">
<title>Find in page2</title>
</head>
<body>
Brad hates prime numbers.
</body>
</html>

1 comment on commit 170a14e

@diracdeltas
Copy link
Member

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.