Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix contentscript injection failure on Firefox 56 #10034

Merged
merged 1 commit into from
Dec 10, 2020

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Dec 10, 2020

On Firefox 56 and Waterfox Classic, our runLockdown.js script throws an error. This is fine on the HTML pages, as the next script tags still get run without issue (though they don't benefit from the SES lockdown sadly). But in the contentscript, an exception thrown here appears to halt the execution of subsequent scripts.

To prevent the contentscript from crashing completely, lockdown errors are now caught and logged. They are also logged to Sentry on the pages where Sentry is setup.

On Firefox 56 and Waterfox Classic, our `runLockdown.js` script throws
an error. This is fine on the HTML pages, as the next script tags still
get run without issue (though they don't benefit from the SES lockdown
sadly). But in the `contentscript`, an exception thrown here appears to
halt the execution of subsequent scripts.

To prevent the `contentscript` from crashing completely, lockdown
errors are now caught and logged. They are also logged to Sentry on the
pages where Sentry is setup.
@Gudahtt Gudahtt requested a review from a team as a code owner December 10, 2020 06:41
@Gudahtt Gudahtt requested a review from brad-decker December 10, 2020 06:41
@metamaskbot
Copy link
Collaborator

Builds ready [23c709e]
Page Load Metrics (527 ± 31 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint316848105
domContentLoaded3365815266431
load3385835276431
domInteractive3365815256431

@brad-decker brad-decker self-assigned this Dec 10, 2020
Copy link
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

LGTM

@Gudahtt Gudahtt merged commit 86fba2d into develop Dec 10, 2020
@Gudahtt Gudahtt deleted the continue-if-lockdown-fails branch December 10, 2020 17:33
@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants