Skip to content

Commit

Permalink
Clear beforeunload handler after button is pressed (#7346)
Browse files Browse the repository at this point in the history
On the signature request and transaction confirmation notification
pages, the closure of the notification UI implies that the request has
been rejected. However, this rejection is being submitted even when the
window is closed as a result of the user explicitly confirming or
rejecting. In practice, I suspect this has no effect because the
transaction, after being explicitly confirmed or rejected, has already
been moved out of a pending state. But just in case there is some
present or future edge case that might be affected, the `beforeunload`
handler is now removed once the user has explicitly made a choice.

This mistake was introduced recently in #7333
  • Loading branch information
Gudahtt authored Nov 4, 2019
1 parent 6a4df0d commit dbd14d7
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 34 deletions.
37 changes: 22 additions & 15 deletions ui/app/components/app/signature-request-original.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,31 +103,36 @@ function SignatureRequest (props) {
}
}

SignatureRequest.prototype.componentDidMount = function () {
SignatureRequest.prototype._beforeUnload = (event) => {
const { clearConfirmTransaction, cancel } = this.props
const { metricsEvent } = this.context
metricsEvent({
eventOpts: {
category: 'Transactions',
action: 'Sign Request',
name: 'Cancel Sig Request Via Notification Close',
},
})
clearConfirmTransaction()
cancel(event)
}

SignatureRequest.prototype._removeBeforeUnload = () => {
if (getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_NOTIFICATION) {
this._onBeforeUnload = event => {
metricsEvent({
eventOpts: {
category: 'Transactions',
action: 'Sign Request',
name: 'Cancel Sig Request Via Notification Close',
},
})
clearConfirmTransaction()
cancel(event)
}
window.addEventListener('beforeunload', this._onBeforeUnload)
window.removeEventListener('beforeunload', this._beforeUnload)
}
}

SignatureRequest.prototype.componentWillUnmount = function () {
SignatureRequest.prototype.componentDidMount = function () {
if (getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_NOTIFICATION) {
window.removeEventListener('beforeunload', this._onBeforeUnload)
window.addEventListener('beforeunload', this._beforeUnload)
}
}

SignatureRequest.prototype.componentWillUnmount = function () {
this._removeBeforeUnload()
}

SignatureRequest.prototype.renderHeader = function () {
return h('div.request-signature__header', [

Expand Down Expand Up @@ -297,6 +302,7 @@ SignatureRequest.prototype.renderFooter = function () {
large: true,
className: 'request-signature__footer__cancel-button',
onClick: event => {
this._removeBeforeUnload()
cancel(event).then(() => {
this.context.metricsEvent({
eventOpts: {
Expand All @@ -315,6 +321,7 @@ SignatureRequest.prototype.renderFooter = function () {
large: true,
className: 'request-signature__footer__sign-button',
onClick: event => {
this._removeBeforeUnload()
sign(event).then(() => {
this.context.metricsEvent({
eventOpts: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,8 @@ export default class ConfirmTransactionBase extends Component {

showRejectTransactionsConfirmationModal({
unapprovedTxCount,
async onSubmit () {
onSubmit: async () => {
this._removeBeforeUnload()
await cancelAllTransactions()
clearConfirmTransaction()
history.push(DEFAULT_ROUTE)
Expand All @@ -409,6 +410,7 @@ export default class ConfirmTransactionBase extends Component {
updateCustomNonce,
} = this.props

this._removeBeforeUnload()
metricsEvent({
eventOpts: {
category: 'Transactions',
Expand Down Expand Up @@ -458,6 +460,7 @@ export default class ConfirmTransactionBase extends Component {
submitting: true,
submitError: null,
}, () => {
this._removeBeforeUnload()
metricsEvent({
eventOpts: {
category: 'Transactions',
Expand Down Expand Up @@ -568,8 +571,30 @@ export default class ConfirmTransactionBase extends Component {
}
}

_beforeUnload = () => {
const { txData: { origin, id } = {}, cancelTransaction } = this.props
const { metricsEvent } = this.context
metricsEvent({
eventOpts: {
category: 'Transactions',
action: 'Confirm Screen',
name: 'Cancel Tx Via Notification Close',
},
customVariables: {
origin,
},
})
cancelTransaction({ id })
}

_removeBeforeUnload = () => {
if (getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_NOTIFICATION) {
window.removeEventListener('beforeunload', this._beforeUnload)
}
}

componentDidMount () {
const { toAddress, txData: { origin, id } = {}, cancelTransaction, getNextNonce, tryReverseResolveAddress } = this.props
const { toAddress, txData: { origin } = {}, getNextNonce, tryReverseResolveAddress } = this.props
const { metricsEvent } = this.context
metricsEvent({
eventOpts: {
Expand All @@ -583,30 +608,15 @@ export default class ConfirmTransactionBase extends Component {
})

if (getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_NOTIFICATION) {
this._onBeforeUnload = () => {
metricsEvent({
eventOpts: {
category: 'Transactions',
action: 'Confirm Screen',
name: 'Cancel Tx Via Notification Close',
},
customVariables: {
origin,
},
})
cancelTransaction({ id })
}
window.addEventListener('beforeunload', this._onBeforeUnload)
window.addEventListener('beforeunload', this._beforeUnload)
}

getNextNonce()
tryReverseResolveAddress(toAddress)
}

componentWillUnmount () {
if (getEnvironmentType(window.location.href) === ENVIRONMENT_TYPE_NOTIFICATION) {
window.removeEventListener('beforeunload', this._onBeforeUnload)
}
this._removeBeforeUnload()
}

render () {
Expand Down

0 comments on commit dbd14d7

Please sign in to comment.