Skip to content

[MV3 Debug Extension] Extension sets the ide query parameter for the DevTools URI #1943

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

Merged
merged 3 commits into from
Feb 8, 2023

Conversation

elliette
Copy link
Contributor

@elliette elliette commented Feb 7, 2023

In the MV2 extension, DWDS could tell whether a DevToolsRequest was for embedding in Chrome DevTools (uriOnly=true). It would then set the ide query parameter to ChromeDevTools if embedded, or DebugExtension if not. See #1522

In the MV3 extension, all DevToolsRequests have uriOnly=true. Therefore, we handle adding the ide query parameter from the extension and not from DWDS.

@elliette elliette changed the title Update how we set query parameters for the DevTools URI [MV3 Debug Extension] Extension sets the ide query parameter for the DevTools URI Feb 7, 2023
@elliette elliette requested a review from annagrin February 7, 2023 22:08
Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Elliott, left a couple of comments

bool? get uriOnly;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious - do they have to be nullable, or can we made them non-nulllable and false by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they make sense to be nullable, since they aren't applicable to non-extension requests. This is the same as tabUrl and contextId (above) which are also null. Added a comment.

// parameter to the DevTools URI:
final devToolsUri = (devToolsRequest.isMv3Extension ?? false)
? _constructDevToolsUri(encodedUri)
: _constructDevToolsUri(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the original extension also add the IDE query parameter, so we don't have to split? Or is it too much change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would have to do a new release of the old extension, and still handle transition period while it is being released (so there would still be a split). I've added a TODO to remove the old code once the MV3 extension is released.

@elliette elliette merged commit 47287e3 into dart-lang:master Feb 8, 2023
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