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
feat: Add support to callback in protocol interceptor handler without argument to perform as if no interceptors. #41929
base: main
Are you sure you want to change the base?
Conversation
💖 Thanks for opening this pull request! 💖 We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix. Examples of commit messages with semantic prefixes:
Things that will help get your PR across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is an user-facing API change, we'll need to update API documentation as well as add tests to ensure coverage for this change.
7867b60
to
b733e96
Compare
Okay. I'll write some tests for it. |
@codebytere @nornagon I've add some test for protocol.interceptProtocol api. But It is a private api so I don't know how to write document for it. But It is ok to work with calling protocol.interceptHttpProtocol api. protocol.interceptHttpProtocol('https', (req, callback) => {
// a Electron.ProtocolResponse or undefined or null or {};
const maybeSpecialRes = getSpecialRes(req); // null or {url, uploadData}
// If maybeSpecialRes is not a Electron.ProtocolResponse then the behavior should perform like no interceptor.
callback(maybeSpecialRes);
}) But the interceptHttpProtocol api forces you to send only http.But protocol.interceptProtocol has more exciting features. protocol.interceptProtocol('https', (req, callback) => {
if (/^https:\/\/docs\.qq\.com\/desktop($|\?|\/)/.test(req.url)) {
// Map request to local file.
callback({
path: path.join(__dirname, 'index.html')
})
return
}
// Perform default http request behavior.
callback();
}); |
469b76d
to
e051b5d
Compare
ElectronURLLoaderFactory can be used for protocol intercept and protocol register. There are much differences. If start loading was called by intercept, the receiver is ProxyingURLLoaderFactory, so you can call remote to trigger it. But if it was called by protocol register, you can't call remote to attempt to take the benefit of the default behavior, the receiver is its self, that will lead to death loop. We should treat intercept and register differently. |
8853a60
to
d6b731f
Compare
d6b731f
to
897732e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codebytere @nornagon If you have time, please could you review my code?
I've done my changes. I think I'm back on the right way to support that you can call callback() to call the Remote created in the ElectronBrowserClient::WillCreateURLLoaderFactory only if you are intercepting a protocol. If the new protocol you registered you can't, you have to call callback function with argument.
I changed the ProxingURLLoaderFactory to treat intercept differently from register a new protocol and updated test files.
And I see a test of webview-spec.ts failed. I think it's irrelevant.
b9196c2
to
8a667f0
Compare
Hi @mrchaofan
Please just made relevant changes inside https://github.com/electron/electron/tree/main/docs/api 👍 |
9493c25
to
2031c18
Compare
@Imperat I've just updated the document and It seems electron.d.ts was automatically updated too. |
… argument to perform as if no interceptors.
2031c18
to
f21ba84
Compare
Description of Change
To SAVE your life while doing protocol intercept.
These changes were made to allow you writting code blow.
Just DO NOT use the documented way.
The document's code is perfect but in real it has tons of bug.
problems
uploadData
inprotocol.handle()
#41052)cc: @codebytere @nornagon
Checklist
npm test
passesRelease Notes
notes: Add support to callback in protocol interceptor handler without argument to perform as if no interceptors.