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

Syntax error in onTabReplaced() #304

Closed
Procyon-b opened this issue Dec 20, 2022 · 8 comments · Fixed by #305
Closed

Syntax error in onTabReplaced() #304

Procyon-b opened this issue Dec 20, 2022 · 8 comments · Fixed by #305
Assignees
Labels
bug confirmed Bugs that have been reproduced by one of the developers

Comments

@Procyon-b
Copy link
Contributor

Procyon-b commented Dec 20, 2022

Item 1

In the final code, line 37839 of main.js, in the function onTabReplaced()

const errmsg = M.react_onTabReplaced(addedTabID, removedTabId);

addedTabID is undefined. It should be addedTabId

Item 2

I'm running 2 instances of TF and the fixed version works correctly, but then I also see an error once on line 37847 where it tells me that tab_val is undefined. I've fixed it temporarily with a try-catch.

@cxw42 cxw42 self-assigned this Dec 22, 2022
@cxw42
Copy link
Owner

cxw42 commented Dec 22, 2022

Thanks for reporting! Would you please try branch issue304 @ 5f62e0a and confirm the issue is resolved? I don't have a good repro for onTabReplaced handy :) . Much appreciated!

@cxw42 cxw42 added the bug label Dec 22, 2022
@cxw42 cxw42 mentioned this issue Dec 22, 2022
1 task
@Procyon-b
Copy link
Contributor Author

Procyon-b commented Dec 22, 2022

I'm now running 0.3.1.1 in parallel. I'll know if it works properly in a few hours.

Edit: One of the computers I'm using has only 2G of ram. It's not a problem with the old win7. And with ublock running in my browsers the webpages are much lighter. So the browser is still perfectly functionnal. It starts swapping when I reach 15-20 tabs (or less if they are big pages).

@Procyon-b
Copy link
Contributor Author

Procyon-b commented Dec 23, 2022

The fix works. I don't have orphaned tabs left in TF list.
But there is still the second error. I've launched chrome with TF 0.3.0 and 0.3.1.1 and looked at the dev tools. Then I opened as many sites as needed to trigger a tabReplace. What I could see is that for every error in 0.3.0 I can see an error in 0.3.1.1

in 0.3.0

Error in event handler: ReferenceError: addedTabID is not defined
    at onTabReplaced (chrome-extension://oflhofndbpjhlfnkbclmfkchpnkghflg/win/main.js:37839:42)

in 0.3.1.1:

Error in event handler: ReferenceError: tab_val is not defined
    at onTabReplaced (chrome-extension://oflhofndbpjhlfnkbclmfkchpnkghflg/win/main.js:37844:75)

@cxw42
Copy link
Owner

cxw42 commented Dec 25, 2022

Sorry --- missed that second item (now marked "Item 2") when I read your initial report! Fixed in force-pushed issue304, now 0.3.1.2. Please let me know if the issue is fixed at 98e8924. I tested the changes by manually invoking the onTabReplaced() function and it at least runs to completion :) . Thanks!

@cxw42 cxw42 added the confirmed Bugs that have been reproduced by one of the developers label Dec 25, 2022
@Procyon-b
Copy link
Contributor Author

I can confirm. Comparing side by side:
No more errors. No orphaned tabs in the list. Looks good. :)

@cxw42
Copy link
Owner

cxw42 commented Dec 25, 2022

@Procyon-b Much appreciated! Will release shortly.

@cxw42
Copy link
Owner

cxw42 commented Dec 25, 2022

0.3.1 submitted for Google review --- will tag when it is released.

@Procyon-b
Copy link
Contributor Author

It's released. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug confirmed Bugs that have been reproduced by one of the developers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants