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

fix: basic repair context-menu script broken #745

Merged
merged 1 commit into from
Oct 27, 2024

Conversation

ACTCD
Copy link
Collaborator

@ACTCD ACTCD commented Oct 26, 2024

fix #743

In old Safari browser.menus.remove just failed silently for non-existent menu item, but in the latest version it rejects the promise, causes failed functional due to unhandled throw.

// since it's not possible to get a list of currently active menu items
// on update, all context-menu items are cleared, then re-added
// this is done to ensure fresh code changes appear
await browser.menus.removeAll();

Since all menu items are actually deleted every time, sessionStorage related logic is not necessary. So simply delete it.

Paths ending in / corresponding to different Match patterns are expected and can be avoided at the user level. There no need to handle it at the extension level, so the relevant logic is deleted.

Anyway, it's just a simple fix to get it basically working again.
But this feature needs to be completely redesigned and refactored.

@ACTCD ACTCD requested a review from quoid October 26, 2024 20:39
Base automatically changed from update-dependencies to main October 27, 2024 02:49
@ACTCD ACTCD merged commit a54946b into main Oct 27, 2024
1 check passed
@ACTCD ACTCD deleted the fix-issue-743-context-menu-script-broken branch October 27, 2024 02:50
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.

Need to restart extension (or safari) to make context-menu script work
2 participants