fix(desktop): give the renderer a platform-native path bridge (#5683,… · marktext/marktext@127c5bb · GitHub
Skip to content

Commit 127c5bb

Browse files
JocsJocs
authored
fix(desktop): give the renderer a platform-native path bridge (#5683, #5669) (#5691)
Follow-up to #5689. Rather than comparing mixed-separator paths, remove the mismatch at its source: the renderer and its preload now use a real Windows path module, so every path they build matches the native paths from the watcher, the main process and the OS. - add `createPlatformPath(isWindows)` (`@jsr/std__path` win32 on Windows, `pathe` on POSIX) and use it for the preload's `window.path` - alias the bare `path` specifier in the renderer build to the same platform path, so shared helpers like `common/envPaths` stop producing `/` paths on Windows - title bar breadcrumb: split on the platform separator — fixes #5669, where a native `C:\…` path was split on `/` and only the filename showed - replace the last hardcoded `/` filesystem join (`CREATE_FILE_DIRECTORY`) - drop the #5689 separator patches (`isSamePath` comparisons, `isPathWithinRoot` folding, `toNativePath`) and the specs that encoded the mixed-separator state `platform-path-windows.spec.ts` drives the real stores with a win32 path module and asserts renderer-built paths are byte-identical to the watcher/main paths, so plain `===` matches — the gate for dropping the patches.
1 parent fbdff9c commit 127c5bb

27 files changed

Lines changed: 414 additions & 405 deletions

‎.npmrc‎

Lines changed: 2 additions & 1 deletion

‎packages/desktop/electron.vite.config.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ export default defineConfig({
4343
preload: {
4444
// --> Bundled as CommonJS
4545
// With sandbox: true the renderer's preload can only `require('electron')`
46-
// (plus a few built-ins). Inline `pathe` (ESM-only) so the bundled preload
47-
// doesn't try to require it from node_modules at runtime.
46+
// (plus a few built-ins). Inline `pathe` and `@std/path` (ESM-only) so the
47+
// bundled preload doesn't try to require them from node_modules at runtime.
4848
build: {
4949
externalizeDeps: {
50-
exclude: ['pathe']
50+
exclude: ['pathe', '@jsr/std__path']
5151
}
5252
},
5353
resolve: {
@@ -80,7 +80,10 @@ export default defineConfig({
8080
'@': resolve(__dirname, 'src/renderer/src'),
8181
common: resolve(__dirname, 'src/common'),
8282
'@shared': resolve(__dirname, 'src/shared'),
83-
path: 'pathe'
83+
// Shared `common/*` helpers import the bare `path` specifier; route it to
84+
// a platform-correct module so the renderer never builds mixed-separator
85+
// paths (#5683).
86+
path: resolve(__dirname, 'src/renderer/src/node/platformPath.ts')
8487
},
8588
extensions: ['.mjs', '.ts', '.js', '.json', '.vue']
8689
},

‎packages/desktop/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
"@element-plus/icons-vue": "^2.3.2",
5858
"@hfelix/electron-localshortcut": "^4.0.1",
5959
"@intlify/core-base": "^11.4.13",
60+
"@jsr/std__path": "^1.1.6",
6061
"@marktext/file-icons": "^1.0.6",
6162
"@muyajs/core": "workspace:*",
6263
"@vscode/ripgrep": "^1.18.0",

‎packages/desktop/src/common/filesystem/paths.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -163,16 +163,6 @@ export const isChildOfDirectory = (dir: string, child: string): boolean => {
163163
return !!relative && !relative.startsWith('..') && !path.isAbsolute(relative)
164164
}
165165

166-
/**
167-
* Convert a renderer path to the OS-native separator before handing it to a
168-
* shell API. The sandboxed renderer's `path` shim only ever emits `/`
169-
* (`src/preload/index.ts`), but Windows' `SHCreateItemFromParsingName` — used
170-
* by `shell.trashItem` / `shell.showItemInFolder` — rejects forward slashes
171-
* with "Failed to parse path" (#5683). `sep` is injectable for tests.
172-
*/
173-
export const toNativePath = (pathname: string, sep: string = path.sep): string =>
174-
sep === '/' ? pathname : path.win32.normalize(pathname)
175-
176166
/**
177167
* Returns true if the pathname matches one of the exclude patterns.
178168
*/

‎packages/desktop/src/common/filesystem/platformPath.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import pathe from 'pathe'
2+
import * as win32 from '@jsr/std__path/windows'
3+
4+
// Node-`path`-shaped module. The sandboxed renderer and its preload cannot load
5+
// node's `path`, and a POSIX-only shim (`pathe`) makes the renderer build `/`
6+
// paths while the watcher, the main process and the OS shell use `\` on
7+
// Windows. Picking the real win32 implementation keeps a single path
8+
// representation (#5683).
9+
export interface PlatformPath {
10+
basename(path: string, ext?: string): string
11+
dirname(path: string): string
12+
extname(path: string): string
13+
join(...paths: string[]): string
14+
resolve(...paths: string[]): string
15+
relative(from: string, to: string): string
16+
isAbsolute(path: string): boolean
17+
normalize(path: string): string
18+
parse(path: string): { root: string, dir: string, base: string, ext: string, name: string }
19+
format(pathObject: {
20+
root?: string
21+
dir?: string
22+
base?: string
23+
ext?: string
24+
name?: string
25+
}): string
26+
sep: string
27+
delimiter: string
28+
}
29+
30+
const posixPath: PlatformPath = {
31+
basename: (path, ext) => pathe.basename(path, ext),
32+
dirname: (path) => pathe.dirname(path),
33+
extname: (path) => pathe.extname(path),
34+
join: (...paths) => pathe.join(...paths),
35+
resolve: (...paths) => pathe.resolve(...paths),
36+
relative: (from, to) => pathe.relative(from, to),
37+
isAbsolute: (path) => pathe.isAbsolute(path),
38+
normalize: (path) => pathe.normalize(path),
39+
parse: (path) => pathe.parse(path),
40+
format: (pathObject) => pathe.format(pathObject),
41+
sep: pathe.sep,
42+
delimiter: pathe.delimiter
43+
}
44+
45+
const win32Path: PlatformPath = {
46+
basename: (path, ext) => win32.basename(path, ext),
47+
dirname: (path) => win32.dirname(path),
48+
extname: (path) => win32.extname(path),
49+
join: (...paths) => win32.join(...paths),
50+
resolve: (...paths) => win32.resolve(...paths),
51+
relative: (from, to) => win32.relative(from, to),
52+
isAbsolute: (path) => win32.isAbsolute(path),
53+
normalize: (path) => win32.normalize(path),
54+
parse: (path) => win32.parse(path),
55+
format: (pathObject) => win32.format(pathObject),
56+
sep: win32.SEPARATOR,
57+
delimiter: win32.DELIMITER
58+
}
59+
60+
export const createPlatformPath = (isWindows: boolean): PlatformPath =>
61+
isWindows ? win32Path : posixPath

‎packages/desktop/src/main/filesystem/trash.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import path from 'path'
22
import { dialog, shell } from 'electron'
33
import type { BrowserWindow } from 'electron'
44
import { t } from '../i18n'
5-
import { toNativePath } from 'common/filesystem/paths'
65

76
// A destructive action must not be the default button, so cancel doubles as
87
// `defaultId` and `cancelId` (Esc cancels).
@@ -14,20 +13,19 @@ export const confirmAndTrashItem = async(
1413
win: BrowserWindow | null,
1514
fullPath: string
1615
): Promise<boolean> => {
17-
const nativePath = toNativePath(fullPath)
1816
const options = {
1917
type: 'warning' as const,
2018
buttons: [t('contextMenu.sideBar.moveToTrash'), t('dialog.cancel')],
2119
defaultId: CANCEL_BUTTON_ID,
2220
cancelId: CANCEL_BUTTON_ID,
2321
noLink: true,
24-
message: t('dialog.moveToTrash', { name: path.basename(nativePath) })
22+
message: t('dialog.moveToTrash', { name: path.basename(fullPath) })
2523
}
2624
const { response } = win
2725
? await dialog.showMessageBox(win, options)
2826
: await dialog.showMessageBox(options)
2927

3028
if (response !== CONFIRM_BUTTON_ID) return false
31-
await shell.trashItem(nativePath)
29+
await shell.trashItem(fullPath)
3230
return true
3331
}

‎packages/desktop/src/main/ipc/shell.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { ipcMain, shell, clipboard, nativeImage } from 'electron'
22
import log from 'electron-log'
33
import * as plist from 'plist'
4-
import { toNativePath } from 'common/filesystem/paths'
54

65
export const registerShellHandlers = (): void => {
76
ipcMain.handle('mt::shell::open-external', async(_e, url: string) => {
@@ -18,14 +17,14 @@ export const registerShellHandlers = (): void => {
1817
})
1918
ipcMain.on('mt::shell::show-item', (_e, fullPath: string) => {
2019
try {
21-
shell.showItemInFolder(toNativePath(fullPath))
20+
shell.showItemInFolder(fullPath)
2221
} catch (err) {
2322
log.error('shell.showItemInFolder failed:', err)
2423
}
2524
})
2625
ipcMain.handle('mt::shell::open-path', async(_e, fullPath: string) => {
2726
try {
28-
return await shell.openPath(toNativePath(fullPath))
27+
return await shell.openPath(fullPath)
2928
} catch (err) {
3029
log.error('shell.openPath failed:', err)
3130
return String(err instanceof Error ? err.message : err)

‎packages/desktop/src/preload/index.ts‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import { contextBridge, ipcRenderer, webFrame, webUtils } from 'electron'
1010
import type { IpcRendererEvent } from 'electron'
1111
import pathe from 'pathe'
12+
import { createPlatformPath } from 'common/filesystem/platformPath'
1213

1314
import type {
1415
IpcInvokeChannels,
@@ -259,27 +260,12 @@ const electronAPI = {
259260
dialog: dialogAPI
260261
}
261262

262-
// Expose a Node-`path`-compatible API to the renderer. `pathe` is a
263-
// cross-platform reimplementation that always uses `/` separators and works
264-
// inside a sandboxed renderer.
265-
const pathAPI = {
266-
basename: (...args: Parameters<typeof pathe.basename>) => pathe.basename(...args),
267-
dirname: (...args: Parameters<typeof pathe.dirname>) => pathe.dirname(...args),
268-
extname: (...args: Parameters<typeof pathe.extname>) => pathe.extname(...args),
269-
join: (...args: string[]) => pathe.join(...args),
270-
resolve: (...args: string[]) => pathe.resolve(...args),
271-
relative: (...args: Parameters<typeof pathe.relative>) => pathe.relative(...args),
272-
isAbsolute: (...args: Parameters<typeof pathe.isAbsolute>) => pathe.isAbsolute(...args),
273-
normalize: (...args: Parameters<typeof pathe.normalize>) => pathe.normalize(...args),
274-
parse: (...args: Parameters<typeof pathe.parse>) => pathe.parse(...args),
275-
format: (...args: Parameters<typeof pathe.format>) => pathe.format(...args),
276-
sep: pathe.sep,
277-
delimiter: pathe.delimiter
278-
// Note: `pathe.posix` / `pathe.win32` are intentionally not exposed.
279-
// Each contains a self-reference (`pathe.posix.posix === pathe.posix`),
280-
// which breaks structured cloning inside `contextBridge.exposeInMainWorld`.
281-
// No code in this repo reads `window.path.posix` / `window.path.win32`.
282-
}
263+
// The sandboxed renderer cannot load node's `path`, so the bridge supplies a
264+
// platform-correct one (POSIX-only `pathe` would make the renderer build `/`
265+
// paths while the watcher/main/OS use `\` on Windows, #5683).
266+
// `pathe.posix` / `pathe.win32` stay unexposed: each self-references
267+
// (`pathe.posix.posix === pathe.posix`), which breaks `contextBridge` cloning.
268+
const pathAPI = createPlatformPath((bootInfo?.platform || process.platform) === 'win32')
283269

284270
// Bundled third-party packages occasionally read `process.platform` at module
285271
// load time (e.g. @hfelix/electron-localshortcut/src/utils.js). Expose a

‎packages/desktop/src/renderer/src/components/sideBar/trashKey.ts‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,15 @@ export const isTrashShortcut = (key: string, metaKey: boolean, isMac: boolean):
5151
return isMac && metaKey && key === 'Backspace'
5252
}
5353

54-
// Watcher/editor paths can use the OS separator while roots built with
55-
// `window.path` use `/` (#5683), so fold both to one separator before comparing.
56-
const toSlash = (value: string): string => value.replace(/[\\/]+/g, '/')
57-
5854
export const isPathWithinRoot = (
5955
pathname: string,
6056
rootPath: string | undefined,
6157
separator: string
6258
): boolean => {
6359
if (!rootPath) return false
64-
const target = toSlash(pathname)
65-
const root = toSlash(rootPath)
66-
if (target === root) return true
67-
const sep = toSlash(separator) || '/'
68-
const prefix = root.endsWith(sep) ? root : root + sep
69-
return target.startsWith(prefix)
60+
if (pathname === rootPath) return true
61+
const prefix = rootPath.endsWith(separator) ? rootPath : rootPath + separator
62+
return pathname.startsWith(prefix)
7063
}
7164

7265
export const shouldTrashSelection = (context: TrashKeyContext): boolean => {

‎packages/desktop/src/renderer/src/components/sideBar/treeFile.vue‎

Lines changed: 2 additions & 6 deletions

0 commit comments

Comments
 (0)