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

extension(taburl): extension Error: couldn't resolve current tab #5591

Merged
merged 2 commits into from
Jul 3, 2018

Conversation

wardpeet
Copy link
Collaborator

Summary

bugfix

When you run the extension and hit enter immediately afterwards, lighthouse tries to start again which will start on about:blank and fails as it can't get an url from the tab.

Related Issues/PRs

https://github.com/GoogleChrome/lighthouse/issues?q=is%3Aissue+Error%3A+Couldn%27t+resolve+current+tab+is%3Aclosed

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

what a pro! awesome job hunting this down @wardpeet 💯 🥇 👍 🎉

background.loadSettings().then(settings => {
onGenerateReportButtonClick(background, settings);
});
if (!background.isRunning()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

micronit: if (background.isRunning()) return; either here or onGenerateReportButtonClick matches a bit more with what I might expect, but awesome 🔍

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

that's amazing!

@wardpeet
Copy link
Collaborator Author

I used a boolean instead. The smoketest didn't work out so no test :(

@paulirish paulirish merged commit b80e455 into master Jul 3, 2018
@paulirish paulirish deleted the bug/tab-not-found branch July 3, 2018 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants