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.

Sign up for GitHub

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

Jump to bottom

Clicking browser action icon sometimes takes two attempts #1491

Closed
Tracked by #1465
birtles opened this issue Dec 6, 2023 · 0 comments · Fixed by #1492
Closed
Tracked by #1465

Clicking browser action icon sometimes takes two attempts #1491

birtles opened this issue Dec 6, 2023 · 0 comments · Fixed by #1492

Comments

@birtles
Copy link
Member

birtles commented Dec 6, 2023

As per [this review]:

I've noticed recently that when I click the icon, it doesn't turn on. I have to click it twice. Why is that? Is it a bug? A lot of times I thought it's enabled and I hover a word to find out it's not on and have to click it again, it's a hassle really.

This is likely fallout from switching to non-persistent background pages. It sounds bad.
@birtles birtles mentioned this issue Dec 6, 2023
Closed
11 tasks
birtles added a commit that referenced this issue Dec 6, 2023
@birtles
fix: fix tab manager initialization race condition
79dcd0f 
STR:

1. Start the add-on but don't enable it
2. Wait for the download to finish
3. Terminate the background process from about:debugging (or just wait
   for it to terminate)
4. On any regular Web page, enable the add-on by clicking the icon on the
   toolbar

Expected results:

The add-on is enabled immediately.

Actual results:

It takes two clicks.

Analysis:

We'd hit a race condition where we'd call tabManager.init() while the
tabManager was still being initialized. As a result it would update its
this.enabled member to `true` but then the initialization steps would
reset it back to false.

The fix is just to wait for the initialization to complete before trying
to toggle the value.

As far as I can tell this does not affect the ActiveTabManager because
its initialization is synchronous.

Fixes #1491.
@birtles birtles mentioned this issue Dec 6, 2023
Merged
birtles added a commit that referenced this issue Dec 6, 2023
@birtles
fix: fix tab manager initialization race condition
9f8644c 
STR:

1. Start the add-on but don't enable it
2. Wait for the download to finish
3. Terminate the background process from about:debugging (or just wait
   for it to terminate)
4. On any regular Web page, enable the add-on by clicking the icon on the
   toolbar

Expected results:

The add-on is enabled immediately.

Actual results:

It takes two clicks.

Analysis:

We'd hit a race condition where we'd call tabManager.init() while the
tabManager was still being initialized. As a result it would update its
this.enabled member to `true` but then the initialization steps would
reset it back to false.

The fix is just to wait for the initialization to complete before trying
to toggle the value.

As far as I can tell this does not affect the ActiveTabManager because
its initialization is synchronous.

Fixes #1491.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: fix tab manager initialization race condition birchill/10ten-ja-reader
1 participant
@birtles