Skip to content

Commit 3a10ef5

Browse files
aboseabose
committed
fix(mdviewer): fix flaky md editor tests and md-to-html switch regression
- Save scroll position before hiding md iframe (panel close / HTML switch) so it can be restored on reopen. Wrapped in try-catch to handle sandboxed cross-origin iframe access that was causing a SecurityError and breaking the md-to-html preview switch entirely. - Flush (not cancel) pending debounced content-change in handleSwitchFile and handleSetContent so outgoing file edits are preserved in cache, preventing data loss when users switch files within the 50ms debounce window. - Tag mdviewrContentChanged messages with filePath so MarkdownSync can verify the change matches the active document, preventing stale edits from a previous file from modifying the wrong CM document. - Guard MarkdownSync._onIframeContentChanged against stale content changes arriving after document close (null CM check + filePath mismatch check). - Guard doc-cache.saveActiveScrollPos against overwriting saved non-zero scroll with 0 after browser resets scrollTop on hide/show. - Fix _waitForMdPreviewReady in all 4 md test files: increase timeout from default 2s to 5s, re-read editor content each poll iteration so comparison adapts to async content sync that may modify the document.
1 parent 336da42 commit 3a10ef5

9 files changed

Lines changed: 97 additions & 15 deletions

File tree

‎src-mdviewer/src/bridge.js‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { getState, setState } from "./core/state.js";
88
import { setLocale } from "./core/i18n.js";
99
import { marked } from "marked";
1010
import * as docCache from "./core/doc-cache.js";
11-
import { broadcastSelectionStateSync } from "./components/editor.js";
11+
import { broadcastSelectionStateSync, flushPendingContentChange } from "./components/editor.js";
1212

1313
let _syncId = 0;
1414
let _lastReceivedSyncId = -1;
@@ -224,6 +224,9 @@ export function initBridge() {
224224
window.__broadcastSelectionStateForTest = function () {
225225
broadcastSelectionStateSync();
226226
};
227+
window.__saveScrollPos = function () {
228+
docCache.saveActiveScrollPos();
229+
};
227230
window.__triggerContentSync = function () {
228231
const content = document.getElementById("viewer-content");
229232
if (content) {
@@ -512,8 +515,11 @@ export function initBridge() {
512515
entry.mdSrc = markdown;
513516
}
514517
}
515-
// Send cursor position BEFORE the edit for undo restore
516-
sendToParent("mdviewrContentChanged", { markdown, _syncId, cursorPos: _cursorPosBeforeEdit });
518+
// Send cursor position BEFORE the edit for undo restore.
519+
// Include file path so MarkdownSync can verify the change matches the active document.
520+
sendToParent("mdviewrContentChanged", {
521+
markdown, _syncId, cursorPos: _cursorPosBeforeEdit, filePath: activePath
522+
});
517523
_cursorPosDirty = false; // allow cursor tracking again
518524
});
519525

@@ -579,6 +585,7 @@ function handleSetContent(data) {
579585
_baseURL = baseURL;
580586
}
581587

588+
flushPendingContentChange();
582589
_suppressContentChange = true;
583590
const parseResult = parseMarkdownToHTML(markdown);
584591

@@ -668,6 +675,12 @@ function handleSwitchFile(data) {
668675
_baseURL = baseURL;
669676
}
670677

678+
// Flush any pending debounced content-change from the outgoing file's edits
679+
// BEFORE suppressing. This ensures the outgoing file's cache entry and
680+
// currentContent are updated with the latest edits, preventing data loss
681+
// when the user switches files quickly (within the 50ms debounce window).
682+
flushPendingContentChange();
683+
671684
_suppressContentChange = true;
672685

673686
// Suppress scroll-to-line from CM during file switch — the doc cache

‎src-mdviewer/src/components/editor.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,6 +1714,23 @@ function emitContentChange(contentEl) {
17141714
}, CONTENT_CHANGE_DEBOUNCE);
17151715
}
17161716

1717+
/**
1718+
* Flush any pending debounced content-change emission immediately.
1719+
* Called during file switch so the outgoing file's edits are synced
1720+
* to its cache entry and CM document before switching away.
1721+
*/
1722+
export function flushPendingContentChange() {
1723+
if (contentChangeTimer) {
1724+
clearTimeout(contentChangeTimer);
1725+
contentChangeTimer = null;
1726+
const contentEl = document.getElementById("viewer-content");
1727+
if (contentEl) {
1728+
const markdown = convertToMarkdown(contentEl);
1729+
emit("bridge:contentChanged", { markdown });
1730+
}
1731+
}
1732+
}
1733+
17171734
function getContentEl() {
17181735
return document.getElementById("viewer-content");
17191736
}

‎src-mdviewer/src/core/doc-cache.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,10 @@ export function saveActiveScrollPos() {
180180
// — hidden elements report scrollTop = 0 which would destroy the saved value.
181181
if (!viewerContainer.offsetParent && viewerContainer.scrollTop === 0) return;
182182

183+
// Don't overwrite a saved non-zero scroll position with 0 — this happens when
184+
// the browser resets scrollTop after hide/show and the caller hasn't scrolled yet.
185+
if (viewerContainer.scrollTop === 0 && entry.scrollPos > 0) return;
186+
183187
entry.scrollPos = viewerContainer.scrollTop;
184188

185189
// Also save source line for reload scenarios (DOM rebuilt, pixel pos unreliable)

‎src/extensionsIntegrated/Phoenix-live-preview/MarkdownSync.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,22 @@ define(function (require, exports, module) {
536536
return;
537537
}
538538

539+
// Guard against stale content changes arriving after the document was closed.
540+
// This can happen because iframe content changes are debounced (50ms) and
541+
// postMessage is async, so they may arrive after FILE_CLOSE but before deactivate().
542+
const cm = _getCM();
543+
if (!cm) {
544+
return;
545+
}
546+
547+
// Ignore content changes for a different file than the one currently active
548+
// in MarkdownSync. This prevents stale debounced edits from a previous file
549+
// from modifying the wrong CM document after a file switch.
550+
if (data.filePath && _doc && _doc.file &&
551+
data.filePath !== _doc.file.fullPath) {
552+
return;
553+
}
554+
539555
const markdown = data.markdown;
540556
const remoteSyncId = data._syncId;
541557

‎src/extensionsIntegrated/Phoenix-live-preview/main.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,16 @@ define(function (require, exports, module) {
610610
// src. so we delete the node itself to eb thorough.
611611
// Don't destroy the persistent md iframe — just hide it
612612
if ($mdviewrIframe && $iframe[0] === $mdviewrIframe[0]) {
613+
// Save scroll position before hiding — hidden elements lose scrollTop.
614+
// Use try-catch because the sandboxed iframe blocks cross-origin property access.
615+
try {
616+
const mdWin = $mdviewrIframe[0].contentWindow;
617+
if (mdWin && mdWin.__saveScrollPos) {
618+
mdWin.__saveScrollPos();
619+
}
620+
} catch (e) {
621+
// Cross-origin access blocked by sandbox — scroll will use source-line restore
622+
}
613623
MarkdownSync.deactivate();
614624
_isMdviewrActive = false;
615625
_updateLPControlsForMdviewer();
@@ -964,6 +974,16 @@ define(function (require, exports, module) {
964974
// Switching away from mdviewr to non-markdown preview
965975
// Hide the md iframe instead of destroying it so cache is preserved
966976
if(_isMdviewrActive) {
977+
// Save scroll position before hiding — hidden elements lose scrollTop.
978+
// Use try-catch because the sandboxed iframe blocks cross-origin property access.
979+
try {
980+
if ($mdviewrIframe && $mdviewrIframe[0].contentWindow &&
981+
$mdviewrIframe[0].contentWindow.__saveScrollPos) {
982+
$mdviewrIframe[0].contentWindow.__saveScrollPos();
983+
}
984+
} catch (e) {
985+
// Cross-origin access blocked by sandbox — scroll will use source-line restore
986+
}
967987
MarkdownSync.deactivate();
968988
_isMdviewrActive = false;
969989
if ($mdviewrIframe) {

‎test/spec/md-editor-edit-integ-test.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ define(function (require, exports, module) {
7474
}
7575

7676
async function _waitForMdPreviewReady(editor) {
77-
const expectedSrc = editor ? editor.document.getText() : null;
7877
await awaitsFor(() => {
7978
const mdIFrame = _getMdPreviewIFrame();
8079
if (!mdIFrame || mdIFrame.style.display === "none") { return false; }
@@ -84,7 +83,11 @@ define(function (require, exports, module) {
8483
if (win.__isSuppressingContentChange && win.__isSuppressingContentChange()) { return false; }
8584
const content = mdIFrame.contentDocument && mdIFrame.contentDocument.getElementById("viewer-content");
8685
if (!content || content.children.length === 0) { return false; }
87-
if (!EditorManager.getActiveEditor()) { return false; }
86+
const activeEditor = EditorManager.getActiveEditor();
87+
if (!activeEditor) { return false; }
88+
// Re-read editor content each iteration — content sync from a previous
89+
// test's DOM edit can modify the document asynchronously (debounced postMessage).
90+
const expectedSrc = activeEditor.document.getText();
8891
if (expectedSrc) {
8992
const viewerSrc = win.__getCurrentContent && win.__getCurrentContent();
9093
if (viewerSrc !== expectedSrc) { return false; }

‎test/spec/md-editor-edit-more-integ-test.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ define(function (require, exports, module) {
7171
}
7272

7373
async function _waitForMdPreviewReady(editor) {
74-
const expectedSrc = editor ? editor.document.getText() : null;
7574
await awaitsFor(() => {
7675
const mdIFrame = _getMdPreviewIFrame();
7776
if (!mdIFrame || mdIFrame.style.display === "none") { return false; }
@@ -81,13 +80,17 @@ define(function (require, exports, module) {
8180
if (win.__isSuppressingContentChange && win.__isSuppressingContentChange()) { return false; }
8281
const content = mdIFrame.contentDocument && mdIFrame.contentDocument.getElementById("viewer-content");
8382
if (!content || content.children.length === 0) { return false; }
84-
if (!EditorManager.getActiveEditor()) { return false; }
83+
const activeEditor = EditorManager.getActiveEditor();
84+
if (!activeEditor) { return false; }
85+
// Re-read editor content each iteration — content sync from a previous
86+
// test's DOM edit can modify the document asynchronously (debounced postMessage).
87+
const expectedSrc = activeEditor.document.getText();
8588
if (expectedSrc) {
8689
const viewerSrc = win.__getCurrentContent && win.__getCurrentContent();
8790
if (viewerSrc !== expectedSrc) { return false; }
8891
}
8992
return true;
90-
}, "md preview synced with editor content");
93+
}, "md preview synced with editor content", 5000);
9194
}
9295

9396
function _dispatchKeyInMdIframe(key, options) {

‎test/spec/md-editor-integ-test.js‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,6 @@ define(function (require, exports, module) {
199199
* @param {Object} editor - The active Editor instance whose content should be synced to the viewer.
200200
*/
201201
async function _waitForMdPreviewReady(editor) {
202-
const expectedSrc = editor ? editor.document.getText() : null;
203202
await awaitsFor(() => {
204203
const mdIFrame = _getMdPreviewIFrame();
205204
if (!mdIFrame || mdIFrame.style.display === "none") { return false; }
@@ -209,14 +208,18 @@ define(function (require, exports, module) {
209208
if (win.__isSuppressingContentChange && win.__isSuppressingContentChange()) { return false; }
210209
const content = mdIFrame.contentDocument && mdIFrame.contentDocument.getElementById("viewer-content");
211210
if (!content || content.children.length === 0) { return false; }
212-
if (!EditorManager.getActiveEditor()) { return false; }
213-
// Verify the viewer has synced with the editor's content
211+
const activeEditor = EditorManager.getActiveEditor();
212+
if (!activeEditor) { return false; }
213+
// Verify the viewer has synced with the editor's content.
214+
// Re-read editor content each iteration — content sync from a previous
215+
// test's DOM edit can modify the document asynchronously (debounced postMessage).
216+
const expectedSrc = activeEditor.document.getText();
214217
if (expectedSrc) {
215218
const viewerSrc = win.__getCurrentContent && win.__getCurrentContent();
216219
if (viewerSrc !== expectedSrc) { return false; }
217220
}
218221
return true;
219-
}, "md preview synced with editor content");
222+
}, "md preview synced with editor content", 5000);
220223
}
221224

222225
describe("livepreview:Markdown Editor", function () {

‎test/spec/md-editor-table-integ-test.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ define(function (require, exports, module) {
5959

6060

6161
async function _waitForMdPreviewReady(editor) {
62-
const expectedSrc = editor ? editor.document.getText() : null;
6362
await awaitsFor(() => {
6463
const mdIFrame = _getMdPreviewIFrame();
6564
if (!mdIFrame || mdIFrame.style.display === "none") { return false; }
@@ -69,13 +68,17 @@ define(function (require, exports, module) {
6968
if (win.__isSuppressingContentChange && win.__isSuppressingContentChange()) { return false; }
7069
const content = mdIFrame.contentDocument && mdIFrame.contentDocument.getElementById("viewer-content");
7170
if (!content || content.children.length === 0) { return false; }
72-
if (!EditorManager.getActiveEditor()) { return false; }
71+
const activeEditor = EditorManager.getActiveEditor();
72+
if (!activeEditor) { return false; }
73+
// Re-read editor content each iteration — content sync from a previous
74+
// test's DOM edit can modify the document asynchronously (debounced postMessage).
75+
const expectedSrc = activeEditor.document.getText();
7376
if (expectedSrc) {
7477
const viewerSrc = win.__getCurrentContent && win.__getCurrentContent();
7578
if (viewerSrc !== expectedSrc) { return false; }
7679
}
7780
return true;
78-
}, "md preview synced with editor content");
81+
}, "md preview synced with editor content", 5000);
7982
}
8083

8184
describe("livepreview:Markdown Editor Table Editing", function () {

0 commit comments

Comments
 (0)