-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Broken root-cert-stats due to var -> let change #536
Comments
It looks like this is your first issue. Welcome! 👋 One of the project maintainers will be with you as soon as possible. We appreciate your patience. To safeguard the health of the project, please take a moment to read our code of conduct. |
This was done in #484 A difference between if (false) {
var y = 5;
}
console.log(y); // prints undefined
if (false) {
let z = 5;
}
console.log(z); // TypeError: z is not defined @rebloor Could you audit all examples and see if this issue happens more than just the one pointed out here? |
@Rob--W I'll take a look at that |
Tested and found the following: Checked, tested, and OK
Checked issues found and confirmed with test:
|
@Rob--W I've completed a review of the changed files. The only one that has the issue appears to be root-cert-stats. Would you like me to revert the change or should we do something else?
I'll review these further later. |
Yes please revert to var. And prepend a comment that it's referenced from popup.js. Ideally the example wouldn't use getBackgroundPage(), but instead use runtime.sendMessage from popup.js + runtime.onMessage in the background page to send the data back. It's bad practice to use getBackgroundPage(), e.g. in a private browsing window that logic breaks. If you can fix the example to use what I just described, it would be great.
I confirmed by code review that these aren't affected by the var/let change. |
What information was incorrect, unhelpful, or incomplete?
The current root-cert-stats extension throws a "TypeError: can't convert undefined to object" at popup.js 8:22
The popup window always shows "No data to display yet"
About a year ago, someone changed var to let at line 3 of background.js for the variable rootCertStats. As a result, backgroundWindow.rootCertStats is undefined.
Changing the let back to var makes things work again.
What did you expect to see?
No error should be thrown. The popup window should show the stats.
Do you have any supporting links, references, or citations?
No response
Do you have anything more you want to share?
It appears the var -> let change was done without testing. I wonder how many other extensions were broken.
The text was updated successfully, but these errors were encountered: