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

add ping handler #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Vap0r1ze
Copy link

Adds a noop ping command that an external extension could call to detect if nos2x is installed.

@Vap0r1ze
Copy link
Author

@fiatjaf I probably should explain why exactly I think this is needed:

The only way an extension can directly check if an extension is installed is by using the overly-permissive management permission & API.

Right now the only way (that I can think of) to indirectly detect the presence of this extension, is to abuse the if (NO_PERMISSIONS_REQUIRED[type]) check by sending type with a property name that has a truthy value on the default Object prototype (eg: "_proto_", "constructor", "", etc).

This is very hacky and may break if NO_PERMISSIONS_REQUIRED becomes something else like a null-prototype object (eg: export const NO_PERMISSIONS_REQUIRED = Object.create(null), import * as NO_PERMISSIONS_REQUIRED from "./constants"). This PR provides an intended way to achieve this without the worry of breaking in the future.

@fiatjaf
Copy link
Owner

fiatjaf commented Jul 30, 2024

What about checking if window.nostr exists in the browser?

@fiatjaf
Copy link
Owner

fiatjaf commented Jul 30, 2024

I think we can merge this, but if this is the way it should be added to NIP-07 first so other extensions can implement it.

@Vap0r1ze
Copy link
Author

What about checking if window.nostr exists in the browser?

This wouldn't work for background scripts unless they create a content script on a non-extension page first to check for it. An extension I'm working on does this because the websocket connections live in the background script.

I think we can merge this, but if this is the way it should be added to NIP-07 first so other extensions can implement it.

Given that this is the ideal way, are you saying that I actually should make a PR for nostr-protocol/nips#1370?

One thing to note (I mentioned this in the linked issue): if a background script wants to utilize another extension for NIP-07 functionality using the Messaging API, they will need to have their own hardcoded list of extension IDs to do ping checks with. An extension could probably give the user the ability to input an extension ID to get around this limitation.

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.

2 participants