Closed MabryTyson closed 1 year ago
github-actions[bot]
commented
1 year ago 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.
Rob--W
commented
1 year ago This was done in https://github.com/mdn/webextensions-examples/pull/484
A difference between var and let is that let is block-scoped, whereas var is hoisted (https://developer.mozilla.org/en-US/docs/Glossary/Hoisting).
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?
rebloor
commented
1 year ago @Rob--W I'll take a look at that
rebloor
commented
1 year ago Tested and found the following:
Checked, tested, and OK
Checked issues found and confirmed with test:
rebloor
commented
1 year ago @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 did find three extension that I couldn't get to work as expected, but I reverted the change and confirmed that made no difference:
I'll review these further later.
Rob--W
commented
1 year ago @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?
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 did find fourextension that I couldn't get to work as expected, but I reverted the change and confirmed that made no difference:
- menu-accesskey-visible - the access key did not work
- native-messaging - failed to get the "pong" with message that the port was closed
- menu-remove-element - the panel doesn't display.
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.