Repository navigation
fix(protocol): register all custom scheme privileges in one call - #136
Merged
Merged
Conversation
`protocol.registerSchemesAsPrivileged()` replaces the whole privilege table
instead of appending to it. `registerDocumentScheme()` and
`registerPdfjsAssetScheme()` each called it once, so the second call silently
dropped `knownote-doc`'s `supportFetchAPI` privilege. The renderer's
`fetch('knownote-doc://docs/<id>')` then failed with
Fetch API cannot load knownote-doc://docs/...: URL scheme "knownote-doc" is not supported
and every PDF failed to open. Introduced by #131.
Protocol modules now only declare their scheme
(`declareDocumentScheme()` / `declarePdfjsAssetScheme()`); the single call to
Electron lives in `privilegedSchemes.ts::applyPrivilegedSchemes()`, invoked once
from `index.ts` before the app is ready.
`smokeTest` gains a renderer-side fetch of `knownote-doc://`. The existing check
used the main process's `net.fetch`, which works even without the privilege -
that is why the packaged gate stayed green while the reader was broken. The
renderer check creates a hidden window and deliberately does not destroy it:
destroying the last window fires `window-all-closed`, and this app closes the
database on that event, which would abort the remaining checks.
Verification (Linux, Electron 39.8.10):
- headless Electron running the repo's real protocol modules: both
`knownote-doc://` (15306 bytes) and `knownote-asset://` (553 bytes) fetch from a
renderer. With two separate `registerSchemesAsPrivileged()` calls the same
harness returns "ERR Failed to fetch" for `knownote-doc://` - the regression,
reproduced.
- `npm test` 283 pass (2 new: one submission point, all declarations applied).
- `npm run typecheck`, `npm run build`.
- `npm run build:unpack` + `smoke:packaged` 22/22, including the new renderer
check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes the PDF reader, which #131 broke. All custom scheme privileges are now declared by the protocol modules and submitted to Electron in a single
registerSchemesAsPrivileged()call.Why?
protocol.registerSchemesAsPrivileged()replaces the privilege table; it does not append. #131 added a second, separate call:so
knownote-doclostsupportFetchAPIand the renderer'sfetch()on it failed:Every PDF failed to open.
protocol.handleitself keeps working in the main process without the privilege, which is why nothing else noticed.The packaged smoke test stayed green because its
knownote-doc://check used the main process (net.fetch), which does not exercise the renderer-facing privilege. That gap is closed here too.Fixes #135. Regression from #131.
What changed?
src/main/protocol/privilegedSchemes.ts: holds declarations, performs the oneregisterSchemesAsPrivileged()call.documentProtocol.ts/pdfjsAssetProtocol.ts:registerXScheme()→declareXScheme(), which only enqueues; no Electron call.index.ts: declares both schemes, then callsapplyPrivilegedSchemes()once, before the app is ready.smokeTest.ts: a new renderer-side fetch ofknownote-doc://(hidden window, intentionally not destroyed — destroying the last window fireswindow-all-closedand this app closes the database on it).test/privilegedSchemes.test.ts: asserts there is exactly oneregisterSchemesAsPrivileged()call site insrc/and thatindex.tsdeclares and submits both schemes.How was this tested?
registerSchemesAsPrivileged()calls →knownote-doc://returnsERR Failed to fetch;{"knownote-doc":"ok 15306","knownote-asset":"ok 553"}.npm test— 283 pass, 0 fail (2 new).npm run typecheck,npm run build— pass.npm run build:unpack+smoke:packaged— 22/22 checks pass, including the newknownote-doc:// is fetchable from a renderer (scheme privileges registered).Screenshots / recordings
Not applicable.
Checklist
npm run typecheckpasses.npm run buildpasses.Desktop / build changes
npm run build:unpackpasses.npm run smoke:packagedpasses.