Skip to content

Conversation

@iamEvanYT
Copy link
Contributor

Changes

  • Added extensions.handleCrxRequest()
  • Added option registerCrxProtocolInDefaultSession when creating an ElectronChromeExtensions instance
  • Update README to include initialization warning

Problem

When having multiple sessions with multiple instances of ElectronChromeExtensions, using crx in the default session will only result in one of the session's extension icons being able to load.

Solutions

  • Expose extensions.handleCrxRequest() to allow custom handling
  • Add an option to prevent handling it automatically in default session.

Sample Code

// Register CRX protocol in default session
app.whenReady().then(() => {
  protocol.handle("crx", async (request) => {
    const url = URL.parse(request.url);
    if (!url) {
      return new Response("Invalid URL", { status: 404 });
    }

    const partition = url?.searchParams.get("partition");
    if (!partition) {
      return new Response("No partition", { status: 400 });
    }

    const partitionSession = session.fromPartition(partition);

    const extensions = ElectronChromeExtensions.fromSession(partitionSession);
    if (!extensions) {
      return new Response("Extensions not found", { status: 404 });
    }

    return extensions.handleCrxRequest(request);
  });
});

✅ By sending this pull request, I agree to the Contributor License Agreement of this project.

@samuelmaddock
Copy link
Owner

I'd prefer supporting these use cases to require minimal code on behalf of the app author. I've adapted your sample code into a public API which supports a non-standard partition. Would the changes in 58abfc9 work for you?

This change would require all apps to call ElectronChromeExtensions.handleCRXProtocol(session) with the session where <browser-action-list> will be rendered.

The API introduced by #136 could also be reused once that lands.

@iamEvanYT
Copy link
Contributor Author

Yep, seems fine. I would still prefer this commit to the docs to be merged tho. It caused me quite a bit of trouble trying to find out what went wrong when setting it up.

@samuelmaddock
Copy link
Owner

Yep, seems fine. I would still prefer this commit to the docs to be merged tho. It caused me quite a bit of trouble trying to find out what went wrong when setting it up.

Can you file a separate issue for this? That way I can better identify what's wrong and potentially fix it instead of adding the warning.

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