Skip to content

Commit 02e92f2

Browse files
cs50victorcs50victor
authored
fix: unicode path name false exit 0, toggle compact header and force logo in desktop mode (#4)
* fix: validate font metadata outputs after generator success * fix: disable compact header in editor config * fix: show branding logo in desktop offline editor * fix: ensure dummy binary in font test and use rAF for logo visibility Create a temporary allfontsgen stub in tests so CI environments without the real binary do not fail on missing-file errors. Replace ad-hoc setTimeout chains for desktop logo injection with requestAnimationFrame scheduling and MutationObserver hooks for reliable, reactive visibility. * test: add e2e logo header visibility check * refactor: migrate logo test to ESM and improve test descriptions * fix: enable compact header in editor config * fix: ci hangs * fix: logo alignment * fix: use import.meta.dirname instead of import.meta.dir in logo test
1 parent 69df204 commit 02e92f2

File tree

8 files changed

+354
-5
lines changed

8 files changed

+354
-5
lines changed

‎__tests__/generate-office-fonts-path.test.js‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { afterEach, describe, expect, test } from 'bun:test';
2+
import fs from 'fs';
3+
import os from 'os';
4+
import path from 'path';
5+
import { spawnSync } from 'child_process';
6+
7+
const SCRIPT_PATH = path.resolve(import.meta.dir, '..', 'scripts', 'generate_office_fonts.js');
8+
const CONVERTER_DIR = path.resolve(import.meta.dir, '..', 'converter');
9+
const DUMMY_BIN = path.join(CONVERTER_DIR, process.platform === 'win32' ? 'allfontsgen.exe' : 'allfontsgen');
10+
11+
const tempDirs = [];
12+
let createdDummyBin = false;
13+
14+
function makeTempDir(prefix) {
15+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
16+
tempDirs.push(dir);
17+
return dir;
18+
}
19+
20+
function writeMockPreload(dir) {
21+
const preloadPath = path.join(dir, 'mock-spawn.js');
22+
fs.writeFileSync(
23+
preloadPath,
24+
`const childProcess = require('child_process');
25+
const fs = require('fs');
26+
const path = require('path');
27+
28+
childProcess.spawnSync = (_bin, args) => {
29+
const allFontsArg = args.find((arg) => arg.startsWith('--allfonts='));
30+
const selectionArg = args.find((arg) => arg.startsWith('--selection='));
31+
const allFontsPath = allFontsArg ? allFontsArg.slice('--allfonts='.length) : null;
32+
const selectionPath = selectionArg ? selectionArg.slice('--selection='.length) : null;
33+
34+
if (process.env.MOCK_WRITE_OUTPUTS === '1' && allFontsPath && selectionPath) {
35+
fs.mkdirSync(path.dirname(allFontsPath), { recursive: true });
36+
fs.writeFileSync(allFontsPath, 'window.AllFonts = [];');
37+
fs.writeFileSync(selectionPath, Buffer.alloc(0));
38+
}
39+
40+
return { status: Number(process.env.MOCK_EXIT_CODE || 0) };
41+
};
42+
`
43+
);
44+
45+
return preloadPath;
46+
}
47+
48+
function ensureDummyBin() {
49+
if (!fs.existsSync(DUMMY_BIN)) {
50+
fs.mkdirSync(CONVERTER_DIR, { recursive: true });
51+
fs.writeFileSync(DUMMY_BIN, '');
52+
fs.chmodSync(DUMMY_BIN, 0o755);
53+
createdDummyBin = true;
54+
}
55+
}
56+
57+
function runGenerator({ fontDataDir, writeOutputs }) {
58+
ensureDummyBin();
59+
const mockDir = makeTempDir('oo-editors-font-mock-');
60+
const preloadPath = writeMockPreload(mockDir);
61+
62+
const env = {
63+
...process.env,
64+
FONT_DATA_DIR: fontDataDir,
65+
MOCK_EXIT_CODE: '0',
66+
MOCK_WRITE_OUTPUTS: writeOutputs ? '1' : '0',
67+
NODE_OPTIONS: [process.env.NODE_OPTIONS, `--require=${preloadPath}`]
68+
.filter(Boolean)
69+
.join(' '),
70+
};
71+
72+
return spawnSync('node', [SCRIPT_PATH], {
73+
env,
74+
encoding: 'utf8',
75+
});
76+
}
77+
78+
afterEach(() => {
79+
while (tempDirs.length > 0) {
80+
const dir = tempDirs.pop();
81+
fs.rmSync(dir, { recursive: true, force: true });
82+
}
83+
if (createdDummyBin) {
84+
fs.rmSync(DUMMY_BIN, { force: true });
85+
createdDummyBin = false;
86+
}
87+
});
88+
89+
describe('generate_office_fonts path handling', () => {
90+
test('should succeed when FONT_DATA_DIR contains non-ASCII characters', () => {
91+
const root = makeTempDir('oo-editors-fontdata-');
92+
const fontDataDir = path.join(root, 'C', 'Users', 'دانيال', 'AppData', 'Roaming', 'interpreter', 'office-extension-fontdata');
93+
94+
const result = runGenerator({ fontDataDir, writeOutputs: true });
95+
96+
expect(result.status).toBe(0);
97+
expect(fs.existsSync(path.join(fontDataDir, 'AllFonts.js'))).toBe(true);
98+
expect(fs.existsSync(path.join(fontDataDir, 'font_selection.bin'))).toBe(true);
99+
});
100+
101+
test('should fail when generator exits 0 but does not write metadata files', () => {
102+
const root = makeTempDir('oo-editors-fontdata-');
103+
const fontDataDir = path.join(root, 'C', 'Users', 'دانيال', 'AppData', 'Roaming', 'interpreter', 'office-extension-fontdata');
104+
105+
const result = runGenerator({ fontDataDir, writeOutputs: false });
106+
107+
expect(result.status).toBe(1);
108+
expect(result.stderr).toContain('font metadata files were not created');
109+
expect(fs.existsSync(path.join(fontDataDir, 'AllFonts.js'))).toBe(false);
110+
});
111+
});

‎__tests__/logo-header.test.js‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import { chromium } from 'playwright';
2+
import fs from 'fs';
3+
import path from 'path';
4+
import os from 'os';
5+
6+
const SERVER_PORT = Number.parseInt(process.env.SERVER_PORT || '8080', 10);
7+
const SERVER_URL = process.env.SERVER_URL || `http://localhost:${SERVER_PORT}`;
8+
const LOAD_TIMEOUT = Number.parseInt(process.env.LOAD_TIMEOUT || '30000', 10);
9+
const LOGO_TIMEOUT = Number.parseInt(process.env.LOGO_TIMEOUT || '30000', 10);
10+
const DEFAULT_LIST_PATH = path.join(import.meta.dirname, '..', 'test', 'logo-files.txt');
11+
const LIST_PATH = process.env.FILE_LIST || process.argv[2] || DEFAULT_LIST_PATH;
12+
13+
function normalizeListEntry(raw) {
14+
if (!raw) return null;
15+
let value = raw.trim();
16+
if (!value || value.startsWith('#')) return null;
17+
if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) {
18+
value = value.slice(1, -1).trim();
19+
}
20+
if (!value) return null;
21+
if (value === '~') value = os.homedir();
22+
else if (value.startsWith('~/')) value = path.join(os.homedir(), value.slice(2));
23+
if (!path.isAbsolute(value)) value = path.resolve(process.cwd(), value);
24+
return value;
25+
}
26+
27+
function loadFileList(listPath) {
28+
if (!fs.existsSync(listPath)) {
29+
throw new Error(`File list not found: ${listPath}`);
30+
}
31+
const lines = fs.readFileSync(listPath, 'utf8').split(/\r?\n/);
32+
const files = lines.map(normalizeListEntry).filter(Boolean);
33+
if (files.length === 0) {
34+
throw new Error(`File list is empty: ${listPath}`);
35+
}
36+
return files;
37+
}
38+
39+
function assertFilesExist(files) {
40+
const missing = files.filter((filePath) => !fs.existsSync(filePath) || !fs.statSync(filePath).isFile());
41+
if (missing.length) {
42+
throw new Error(`Missing files:\n${missing.join('\n')}`);
43+
}
44+
}
45+
46+
function getDocType(ext) {
47+
if (['xlsx', 'xls', 'ods', 'csv'].includes(ext)) return 'cell';
48+
if (['docx', 'doc', 'odt', 'txt', 'rtf', 'html'].includes(ext)) return 'word';
49+
if (['pptx', 'ppt', 'odp'].includes(ext)) return 'slide';
50+
return 'slide';
51+
}
52+
53+
function buildOfflineLoaderUrl(filePath) {
54+
const ext = path.extname(filePath).slice(1).toLowerCase();
55+
const doctype = getDocType(ext);
56+
const convertUrl = `${SERVER_URL}/api/convert?filepath=${encodeURIComponent(filePath)}`;
57+
const params = new URLSearchParams({
58+
url: convertUrl,
59+
title: path.basename(filePath),
60+
filepath: filePath,
61+
filetype: ext,
62+
doctype,
63+
});
64+
return `${SERVER_URL}/offline-loader-proper.html?${params.toString()}`;
65+
}
66+
67+
async function assertServer() {
68+
const response = await fetch(`${SERVER_URL}/healthcheck`).catch(() => null);
69+
if (!response || !response.ok) {
70+
throw new Error(`Server not running at ${SERVER_URL}. Start with: bun run server`);
71+
}
72+
}
73+
74+
async function assertLogoVisible(page, filePath) {
75+
const url = buildOfflineLoaderUrl(filePath);
76+
await page.goto(url, { waitUntil: 'load', timeout: LOAD_TIMEOUT });
77+
await page.waitForFunction(() => {
78+
const iframe = document.querySelector('iframe[id*="placeholder"]') ||
79+
document.querySelector('iframe[id*="frameEditor"]') ||
80+
document.querySelector('iframe');
81+
if (!iframe || !iframe.contentDocument) return false;
82+
83+
const doc = iframe.contentDocument;
84+
const logo = doc.querySelector('#oo-desktop-logo') ||
85+
doc.querySelector('#box-document-title .extra img[src*="header-logo_s.svg"]') ||
86+
doc.querySelector('#box-document-title img[src*="header-logo_s.svg"]');
87+
if (!logo) return false;
88+
89+
const style = doc.defaultView.getComputedStyle(logo);
90+
const rect = logo.getBoundingClientRect();
91+
return style.display !== 'none' && style.visibility !== 'hidden' && rect.width > 0 && rect.height > 0;
92+
}, { timeout: LOGO_TIMEOUT });
93+
}
94+
95+
async function run() {
96+
await assertServer();
97+
const files = loadFileList(LIST_PATH);
98+
assertFilesExist(files);
99+
100+
const browser = await chromium.launch({ headless: true });
101+
const page = await browser.newPage({ viewport: { width: 1400, height: 900 } });
102+
103+
try {
104+
for (const filePath of files) {
105+
await assertLogoVisible(page, filePath);
106+
console.log(`PASS logo visible: ${filePath}`);
107+
}
108+
console.log(`Logo header check passed for ${files.length} files`);
109+
await browser.close();
110+
process.exit(0);
111+
} catch (err) {
112+
await browser.close();
113+
console.error('Logo header check failed');
114+
console.error(err && err.message ? err.message : err);
115+
process.exit(1);
116+
}
117+
}
118+
119+
run().catch((err) => {
120+
console.error(err && err.message ? err.message : err);
121+
process.exit(1);
122+
});

‎editors/desktop-stub.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,14 +541,19 @@
541541
// Editor configuration - CRITICAL for fixing customization errors
542542
GetEditorConfig: function() {
543543
console.log('[BROWSER] GetEditorConfig called');
544+
var headerLogoUrl = window.location.origin + '/web-apps/apps/common/main/resources/img/header/header-logo_s.svg';
544545
return JSON.stringify({
545546
customization: {
546547
autosave: true,
547548
chat: false,
548549
comments: false,
549550
help: false,
550551
hideRightMenu: false,
551-
compactHeader: true
552+
compactHeader: true,
553+
logo: {
554+
visible: true,
555+
image: headerLogoUrl
556+
}
552557
},
553558
mode: 'edit',
554559
canCoAuthoring: false,

‎editors/offline-loader-proper.html‎

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,83 @@
174174
// Run dialog dismissal every 2 seconds (much less aggressive)
175175
console.log('[DISMISS] Starting dialog dismissal checker');
176176
setInterval(dismissDialogsOnce, 2000);
177+
178+
function ensureDesktopLogoVisible() {
179+
try {
180+
var iframe = document.querySelector('iframe[id*="placeholder"]') ||
181+
document.querySelector('iframe[id*="frameEditor"]') ||
182+
document.querySelector('iframe');
183+
if (!iframe || !iframe.contentDocument) return;
184+
185+
var iframeDoc = iframe.contentDocument;
186+
var logoSrc = window.location.origin + '/web-apps/apps/common/main/resources/img/header/header-logo_s.svg';
187+
var logo = iframeDoc.getElementById('oo-desktop-logo');
188+
if (!logo) {
189+
logo = iframeDoc.createElement('img');
190+
logo.id = 'oo-desktop-logo';
191+
logo.alt = 'ONLYOFFICE';
192+
logo.src = logoSrc;
193+
}
194+
195+
var host = iframeDoc.querySelector('#box-document-title .extra') ||
196+
iframeDoc.querySelector('.extra');
197+
198+
if (host) {
199+
if (logo.parentNode !== host) {
200+
host.insertBefore(logo, host.firstChild);
201+
}
202+
logo.style.cssText = 'height:20px;max-width:120px;margin:2px 12px 0 6px;display:block;pointer-events:none;';
203+
} else {
204+
if (logo.parentNode !== iframeDoc.body) {
205+
iframeDoc.body.appendChild(logo);
206+
}
207+
logo.style.cssText = 'position:fixed;top:12px;left:12px;z-index:2147483647;height:20px;max-width:120px;pointer-events:none;';
208+
}
209+
} catch (e) {
210+
// NOTE(victor): best-effort -- cross-origin iframe access may throw
211+
}
212+
}
213+
214+
function scheduleLogoEnsure(frames) {
215+
var remaining = typeof frames === 'number' ? frames : 4;
216+
function tick() {
217+
ensureDesktopLogoVisible();
218+
remaining -= 1;
219+
if (remaining > 0 && window.requestAnimationFrame) {
220+
window.requestAnimationFrame(tick);
221+
}
222+
}
223+
224+
if (window.requestAnimationFrame) {
225+
window.requestAnimationFrame(tick);
226+
} else {
227+
ensureDesktopLogoVisible();
228+
}
229+
}
230+
231+
function bindLogoRefreshHooks() {
232+
var iframe = document.querySelector('iframe[id*="placeholder"]') ||
233+
document.querySelector('iframe[id*="frameEditor"]') ||
234+
document.querySelector('iframe');
235+
if (!iframe || iframe.dataset.logoHooksBound === 'true') return;
236+
237+
iframe.dataset.logoHooksBound = 'true';
238+
iframe.addEventListener('load', function() {
239+
scheduleLogoEnsure(6);
240+
});
241+
242+
try {
243+
if (iframe.contentDocument && iframe.contentDocument.body && window.MutationObserver) {
244+
var observer = new MutationObserver(function() {
245+
scheduleLogoEnsure(2);
246+
});
247+
observer.observe(iframe.contentDocument.body, { childList: true, subtree: true });
248+
}
249+
} catch (e) {
250+
// NOTE(victor): best-effort -- cross-origin iframe access may throw
251+
}
252+
}
253+
setInterval(ensureDesktopLogoVisible, 3000);
177254
// ========================================================================
178255

179256
// Parse URL parameters
@@ -337,6 +414,7 @@
337414
}
338415

339416
function getEditorConfig(urlParams) {
417+
var headerLogoUrl = window.location.origin + '/web-apps/apps/common/main/resources/img/header/header-logo_s.svg';
340418
return {
341419
customization : {
342420
goback: { url: "onlyoffice.com" },
@@ -347,7 +425,11 @@
347425
showReviewChanges: false,
348426
toolbarNoTabs: false,
349427
uiTheme: 'theme-classic-light', // Prevent theme switching dialogs
350-
autosave: false // Disable autosave completely
428+
autosave: false, // Disable autosave completely
429+
logo: {
430+
visible: true,
431+
image: headerLogoUrl
432+
}
351433
},
352434
mode : urlParams["mode"] || 'edit',
353435
lang : urlParams["lang"] || 'en',
@@ -453,6 +535,10 @@
453535
}
454536
} catch (e) { console.error('[FIX] Error setting iframe ID:', e); } }, 100);
455537

538+
// NOTE(victor): SDK hides the header in desktop mode, so we force-inject the logo
539+
bindLogoRefreshHooks();
540+
scheduleLogoEnsure(8);
541+
456542
// NOTE(victor): Poll every 50ms instead of fixed 5s delay - SDK is usually ready immediately
457543
var SDK_POLL_INTERVAL = 50;
458544
var SDK_MAX_WAIT = 10000;
@@ -628,6 +714,7 @@
628714
console.log('=== DOCUMENT READY ===');
629715
console.log('[TIMING] Document ready! Total time:', (PERF.documentReady - PERF.loaderStart).toFixed(0), 'ms');
630716
logTimings();
717+
scheduleLogoEnsure(8);
631718

632719
// CRITICAL FIX: Initialize change tracking to stop infinite polling
633720
// The SDK polls LocalFileGetSaved() and LocalFileGetOpenChangesCount()

‎package.json‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@
77
"server": "cross-env FONT_DATA_DIR=assets/onlyoffice-fontdata node server.js",
88
"test": "bun run test:unit && bun run test:e2e",
99
"test:all": "bun run test:unit && bun run test:e2e",
10-
"test:unit": "bun test __tests__/server-utils.test.js __tests__/desktop-stub-utils.test.js && node test-url-scheme.js",
11-
"test:e2e": "bun run test:console-batch && bun run test:save",
10+
"test:unit": "bun test __tests__/server-utils.test.js __tests__/desktop-stub-utils.test.js __tests__/generate-office-fonts-path.test.js && node test-url-scheme.js",
11+
"test:e2e": "bun run test:console-batch && bun run test:logo && bun run test:save",
1212
"test:console-batch": "node test-console-batch.js",
13+
"test:logo": "node __tests__/logo-header.test.js",
1314
"test:save": "bun test __tests__/save.test.js",
1415
"build": "cd sdkjs/build && npm install && npx grunt --desktop=true && cd ../.. && for e in cell word slide visio; do mkdir -p editors/sdkjs/$e && cp sdkjs/deploy/sdkjs/$e/sdk-all.js editors/sdkjs/$e/ && cp sdkjs/deploy/sdkjs/$e/sdk-all-min.js editors/sdkjs/$e/; done",
1516
"build:minify": "bunx esbuild server.js --minify --outfile=dist/server.js"

0 commit comments

Comments
 (0)