Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions apps/desktop/electron-builder.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,11 @@ export default {
to: 'bundled-tools.json',
},
{
// The app icon is read at runtime by the BrowserWindow `icon` option, and
// `files` above does not carry `assets/`. Electron reports the missing
// file as an empty image rather than an error, so without this the
// packaged app just draws no window icon.
// The app icon is read at runtime by the BrowserWindow `icon` option
// and by the permission-overlay card, and `files` above does not carry
// `assets/`. Electron reports the missing file as an empty image rather
// than an error, so without this the packaged app just draws no window
// icon; `assertPackagedResources` requires it on current builds.
from: 'assets',
to: 'assets',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ import {
type PermissionOverlayDeps,
type PermissionOverlayWindowLike,
} from '../permission-overlay/permission-overlay-controller.js';
import {
BUNDLE_ICON_OPTIONS,
loadNativeBundleIcon,
resolveAppBundle,
} from '../permission-overlay/app-bundle.js';
import { resolveAppBundle } from '../permission-overlay/app-bundle.js';

/** Deterministic timer wheel — no real time passes in these tests. */
function createClock() {
Expand Down Expand Up @@ -307,23 +303,6 @@ describe('drag-to-grant permission overlay', () => {
});

describe('app bundle resolution for the drag', () => {
it('never calls the native icon loader for an unpackaged app', async () => {
let calls = 0;
const icon = await loadNativeBundleIcon(false, async () => {
calls += 1;
return 'icon';
});
assert.equal(icon, null);
assert.equal(calls, 0, 'unpackaged development must not call app.getFileIcon()');
assert.equal(await loadNativeBundleIcon(true, async () => 'icon'), 'icon');
});

it('never requests the large icon size that kills packaged macOS builds', () => {
// 'large' hits a fatal NOTREACHED inside Chromium's IconLoader on
// macOS (SIGTRAP, not a catchable error) — see issue #3352.
assert.notEqual(BUNDLE_ICON_OPTIONS.size as string, 'large');
});

it('walks three levels up from the executable to the .app', () => {
assert.deepEqual(
resolveAppBundle({
Expand Down
28 changes: 0 additions & 28 deletions apps/desktop/src/main/permission-overlay/app-bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,34 +34,6 @@ export interface ResolveAppBundleDeps {
exists(path: string): boolean;
}

/**
* The only size every platform can actually deliver. `'large'` is
* unsupported on macOS: Chromium's IconLoader hits a fatal NOTREACHED and
* the process dies with SIGTRAP before the promise settles — no JavaScript
* error is ever thrown, so the try/catch in `loadNativeBundleIcon` cannot
* save the app. Callers upscale the 32x32 result as needed.
*/
export const BUNDLE_ICON_OPTIONS = { size: 'normal' } as const;

/**
* Reading a bundle icon is presentation-only. The original unpackaged npm
* Electron runtime could terminate natively while macOS resolved its bundle
* icon, before the returned promise settled. Keep native icon loading for
* packaged Maka.app builds and let the signed Maka Dev workflow use an empty
* drag image instead.
*/
export async function loadNativeBundleIcon<T>(
isPackaged: boolean,
load: () => Promise<T>,
): Promise<T | null> {
if (!isPackaged) return null;
try {
return await load();
} catch {
return null;
}
}

export function resolveAppBundle(deps: ResolveAppBundleDeps): AppBundleResult {
const { executablePath, platform, exists } = deps;
if (platform !== 'darwin') {
Expand Down
46 changes: 23 additions & 23 deletions apps/desktop/src/main/permission-overlay/permission-overlay-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ import { createRequire } from 'node:module';
import { existsSync } from 'node:fs';
import { join } from 'node:path';
import type { UiLocale } from '@maka/core/ui-locale';
import { desktopAssetPath } from '../desktop-assets.js';
import { resolveOverlayAssetDir } from '../overlay-assets.js';
import { openSystemPermissionPane, requestPermissionAccess } from '../permissions-actions.js';
import { BUNDLE_ICON_OPTIONS, loadNativeBundleIcon, resolveAppBundle } from './app-bundle.js';
import { resolveAppBundle } from './app-bundle.js';
import { getPermissionOverlayCopy } from './permission-overlay-copy.js';
import {
createPermissionOverlayController,
Expand Down Expand Up @@ -64,7 +65,7 @@ export function createPermissionOverlayMain(
let locale: UiLocale = 'en';
let iconDataUrl: string | null = null;
const electron = requireElectron('electron') as Electron;
const { BrowserWindow, app, screen, systemPreferences } = electron;
const { BrowserWindow, app, nativeImage, screen, systemPreferences } = electron;

function isGranted(id: DragGrantPermissionId): boolean {
if (process.platform !== 'darwin') return false;
Expand All @@ -74,15 +75,21 @@ export function createPermissionOverlayMain(
return systemPreferences.getMediaAccessStatus('screen') === 'granted';
}

async function resolveAppIconDataUrl(bundlePath: string | null): Promise<string | null> {
if (!bundlePath) return null;
const icon = await loadNativeBundleIcon(app.isPackaged, () =>
app.getFileIcon(bundlePath, BUNDLE_ICON_OPTIONS),
function resolveAppIconDataUrl(): string | null {
// The canonical 1024px icon ships in assets/ — the same PNG
// electron-builder stamps onto the bundle. `app.getFileIcon()` is the
// wrong source for an identity: macOS reduces the path to its UTType
// and returns the generic application icon, never this app's own — and
// requesting it at size 'large' killed packaged builds outright with a
// fatal NOTREACHED inside Chromium's IconLoader (#3352).
const icon = nativeImage.createFromPath(
desktopAssetPath(
{ isPackaged: app.isPackaged, resourcesPath: process.resourcesPath },
'assets',
'icon.png',
),
);
if (!icon || icon.isEmpty()) return null;
// nativeImage.createFromPath does not decode .icns reliably. Asking
// macOS for the bundle icon returns the same image Finder and the TCC
// list display for packaged Maka builds.
if (icon.isEmpty()) return null;
return icon.resize({ width: 64, height: 64 }).toDataURL();
}

Expand Down Expand Up @@ -110,6 +117,10 @@ export function createPermissionOverlayMain(
exists: existsSync,
});
const bundlePath = bundle.ok ? bundle.bundlePath : null;
// Resolved here, at the only consumer, so a non-darwin start() never
// pays the PNG decode; a null result (asset missing) retries on the
// next card rather than caching the failure.
iconDataUrl ??= resolveAppIconDataUrl();
return {
permission: id,
appName: app.getName(),
Expand Down Expand Up @@ -187,12 +198,6 @@ export function createPermissionOverlayMain(
} catch (error) {
console.warn('[permission-overlay] locale lookup failed, keeping', locale, error);
}
const bundle = resolveAppBundle({
executablePath: app.getPath('exe'),
platform: process.platform,
exists: existsSync,
});
iconDataUrl = await resolveAppIconDataUrl(bundle.ok ? bundle.bundlePath : null);
return controller.start(id);
},
};
Expand Down Expand Up @@ -267,18 +272,13 @@ function attachCardGestures(win: import('electron').BrowserWindow): void {
payload && typeof payload === 'object' && 'iconDataUrl' in payload
? (payload as { iconDataUrl?: unknown }).iconDataUrl
: null;
// The file drag still works without a decorative drag image, so a
// failed decode degrades to an empty icon rather than a native read.
let icon = nativeImage.createEmpty();
if (typeof iconDataUrl === 'string' && iconDataUrl.startsWith('data:image/')) {
const fromRenderer = nativeImage.createFromDataURL(iconDataUrl);
if (!fromRenderer.isEmpty()) icon = fromRenderer;
}
if (icon.isEmpty()) {
const fallback = await loadNativeBundleIcon(app.isPackaged, () =>
app.getFileIcon(resolved.bundlePath, BUNDLE_ICON_OPTIONS),
);
if (fallback && !fallback.isEmpty()) icon = fallback.resize({ width: 64, height: 64 });
// The file drag still works without a decorative drag image.
}

if (!win.isDestroyed()) win.webContents.startDrag({ file: resolved.bundlePath, icon });
});
Expand Down
6 changes: 6 additions & 0 deletions scripts/verify-packaged-app.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,11 @@ export async function assertPackagedResources(
// build, which predates the disclaimer being packaged. Requiring it there
// would fail a release that was correct when it shipped.
requireDisclaimer = true,
// Same shape as the disclaimer: the canonical icon began shipping as an
// extra resource with the window-icon fix, and the permission overlay
// reads it at runtime, so current builds must carry it — but a
// previously released baseline predates it.
requireCanonicalIcon = true,
} = {},
) {
if (bundledGitContract !== 'forbidden' && bundledGitContract !== 'legacy-required') {
Expand All @@ -732,6 +737,7 @@ export async function assertPackagedResources(
join('licenses', 'git', 'NOTICE.txt'),
]
: []),
...(requireCanonicalIcon ? [join('assets', 'icon.png')] : []),
join('workers', 'filesystem-worker.js'),
join('licenses', 'maka', 'LICENSE'),
join('licenses', 'maka', 'NOTICE'),
Expand Down
36 changes: 36 additions & 0 deletions scripts/verify-packaged-app.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ test('legacy packaged resources require the historical bundled Git contract', as
forbidPath: async (path) => forbidden.push(path),
requireWindowsSandbox: false,
bundledGitContract: 'legacy-required',
requireCanonicalIcon: false,
});

for (const path of [
Expand Down Expand Up @@ -304,3 +305,38 @@ describe('assertPackagedDependencyClosure', () => {
);
});
});

// The resource list is contract, not implementation: the permission overlay
// reads `assets/icon.png` at runtime, so a current build that drops it ships
// a regression the app cannot report. The check is driven through the
// injectable `requirePath`, so it needs no packaging and no platform.
describe('assertPackagedResources', () => {
const resources = join('fake', 'resources');
const iconPath = join(resources, 'assets', 'icon.png');
const requirePathMissing = (absent) => async (path) => {
if (path === absent) throw new Error(`MISSING ${path}`);
};
const forbidPath = async () => {};

test('a current build must carry the canonical icon', async () => {
await assert.rejects(
() =>
assertPackagedResources(resources, {
requirePath: requirePathMissing(iconPath),
forbidPath,
requireWindowsSandbox: false,
}),
/MISSING .*icon\.png/,
);
});

test('a legacy baseline predating the packaged icon is not required to carry it', async () => {
await assertPackagedResources(resources, {
requirePath: requirePathMissing(iconPath),
forbidPath,
requireWindowsSandbox: false,
requireDisclaimer: false,
requireCanonicalIcon: false,
});
});
});
1 change: 1 addition & 0 deletions scripts/verify-windows-x64.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export async function verifyPackagedWindowsApp(
requireWindowsSandbox: requiresCurrentContract,
requireDisclaimer: requiresCurrentContract,
bundledGitContract: requiresCurrentContract ? 'forbidden' : 'legacy-required',
requireCanonicalIcon: requiresCurrentContract,
});
if (requiresCurrentContract) await assertPackagedDependencyClosure(resources);
else await requirePath(join(resources, 'git', 'cmd', 'git.exe'));
Expand Down