fix(#4346): address drag-release and recycled-row review regressions
# Conflicts: # static/ui.js # tests/test_issue677.py
This commit is contained in:
1 parent
8494576d70
commit
568fb52727
5 files changed
+170
-15
No files matched your search
+17
-4
@@ -972,6 +972,7 @@ function _scheduleMessageVirtualizedRender(force){
|
|||||||
_programmaticScroll=true;
|
_programmaticScroll=true;
|
||||||
_programmaticScrollSetAt=performance.now();
|
_programmaticScrollSetAt=performance.now();
|
||||||
_compensateScrollForMeasurementDelta(()=>{ renderMessages({ preserveScroll:true }); });
|
_compensateScrollForMeasurementDelta(()=>{ renderMessages({ preserveScroll:true }); });
|
||||||
|
_deferClearProgrammaticScroll();
|
||||||
_messageVirtualWindowKey=liveKey;
|
_messageVirtualWindowKey=liveKey;
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -3928,6 +3929,10 @@ if(typeof window!=='undefined'){
|
|||||||
_scrollbarDragActive=false;
|
_scrollbarDragActive=false;
|
||||||
_scheduleMessageVirtualizedRender(true);
|
_scheduleMessageVirtualizedRender(true);
|
||||||
},{passive:true});
|
},{passive:true});
|
||||||
|
window.addEventListener('blur',()=>{ _scrollbarDragActive=false; },{passive:true});
|
||||||
|
document.addEventListener('visibilitychange',()=>{
|
||||||
|
if(document.visibilityState==='hidden') _scrollbarDragActive=false;
|
||||||
|
},{passive:true});
|
||||||
let _scrollRaf=0;
|
let _scrollRaf=0;
|
||||||
el.addEventListener('scroll',()=>{
|
el.addEventListener('scroll',()=>{
|
||||||
_scheduleMessageVirtualizedRender();
|
_scheduleMessageVirtualizedRender();
|
||||||
@@ -11437,17 +11442,19 @@ function renderMessages(options){
|
|||||||
if(isUser){
|
if(isUser){
|
||||||
currentAssistantTurn=null;
|
currentAssistantTurn=null;
|
||||||
let row=_msgNodeRecycleEnabled?_recycleStash.get(rawIdx):null;
|
let row=_msgNodeRecycleEnabled?_recycleStash.get(rawIdx):null;
|
||||||
const newRawText=String(displayContent).trim();
|
|
||||||
if(row&&!row.classList.contains('msg-row')) row=null;
|
if(row&&!row.classList.contains('msg-row')) row=null;
|
||||||
|
const newRawText=String(displayContent).trim();
|
||||||
|
const nextRowHtml=`${filesHtml}<div class="msg-body">${bodyHtml}</div>${footHtml}`;
|
||||||
if(row){
|
if(row){
|
||||||
row.id=_userMessageDomId(rawIdx);
|
row.id=_userMessageDomId(rawIdx);
|
||||||
row.dataset.msgIdx=rawIdx;
|
row.dataset.msgIdx=rawIdx;
|
||||||
row.dataset.sessionMsgIdx=_messageSessionIndexForRawIdx(rawIdx);
|
row.dataset.sessionMsgIdx=_messageSessionIndexForRawIdx(rawIdx);
|
||||||
row.dataset.messageAnchorKey=_messageViewportAnchorKeyForMessage(m);
|
row.dataset.messageAnchorKey=_messageViewportAnchorKeyForMessage(m);
|
||||||
row.dataset.role='user';
|
row.dataset.role='user';
|
||||||
if(row.dataset.rawText!==newRawText){
|
delete row.dataset.editing;
|
||||||
|
if(row.dataset.rawText!==newRawText||row.innerHTML!==nextRowHtml){
|
||||||
row.dataset.rawText=newRawText;
|
row.dataset.rawText=newRawText;
|
||||||
row.innerHTML=`${filesHtml}<div class="msg-body">${bodyHtml}</div>${footHtml}`;
|
row.innerHTML=nextRowHtml;
|
||||||
}
|
}
|
||||||
}else{
|
}else{
|
||||||
row=document.createElement('div');
|
row=document.createElement('div');
|
||||||
@@ -11458,7 +11465,7 @@ function renderMessages(options){
|
|||||||
row.dataset.messageAnchorKey=_messageViewportAnchorKeyForMessage(m);
|
row.dataset.messageAnchorKey=_messageViewportAnchorKeyForMessage(m);
|
||||||
row.dataset.role='user';
|
row.dataset.role='user';
|
||||||
row.dataset.rawText=newRawText;
|
row.dataset.rawText=newRawText;
|
||||||
row.innerHTML=`${filesHtml}<div class="msg-body">${bodyHtml}</div>${footHtml}`;
|
row.innerHTML=nextRowHtml;
|
||||||
}
|
}
|
||||||
inner.appendChild(row);
|
inner.appendChild(row);
|
||||||
userRows.set(rawIdx, row);
|
userRows.set(rawIdx, row);
|
||||||
@@ -11471,10 +11478,16 @@ function renderMessages(options){
|
|||||||
if(recycled){
|
if(recycled){
|
||||||
const blocks=_assistantTurnBlocks(recycled);
|
const blocks=_assistantTurnBlocks(recycled);
|
||||||
if(blocks) blocks.innerHTML='';
|
if(blocks) blocks.innerHTML='';
|
||||||
|
recycled.removeAttribute('data-transparent-turn-collapsed');
|
||||||
|
recycled.removeAttribute('data-transparent-turn-toggle-bound');
|
||||||
|
const role=recycled.querySelector('.msg-role.assistant');
|
||||||
|
if(role) role.outerHTML=_assistantRoleHtml(tsTitle, isTpsDisplayEnabled()?_formatTurnTps(m._turnTps):'');
|
||||||
currentAssistantTurn=recycled;
|
currentAssistantTurn=recycled;
|
||||||
}else{
|
}else{
|
||||||
currentAssistantTurn=_createAssistantTurn(tsTitle, isTpsDisplayEnabled()?_formatTurnTps(m._turnTps):'');
|
currentAssistantTurn=_createAssistantTurn(tsTitle, isTpsDisplayEnabled()?_formatTurnTps(m._turnTps):'');
|
||||||
}
|
}
|
||||||
|
currentAssistantTurn.dataset.role='assistant';
|
||||||
|
if(S.session) currentAssistantTurn.dataset.sessionId=S.session.session_id;
|
||||||
currentAssistantTurn.dataset.recycleKey=rawIdx;
|
currentAssistantTurn.dataset.recycleKey=rawIdx;
|
||||||
inner.appendChild(currentAssistantTurn);
|
inner.appendChild(currentAssistantTurn);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -28,7 +28,8 @@ def test_attached_files_context_is_hidden_from_user_message_display():
|
|||||||
|
|
||||||
assert "function _stripAttachedFilesMarkerForDisplay" in ui_src
|
assert "function _stripAttachedFilesMarkerForDisplay" in ui_src
|
||||||
assert "_stripAttachedFilesMarkerForDisplay(_stripWorkspaceDisplayPrefix(content))" in ui_src
|
assert "_stripAttachedFilesMarkerForDisplay(_stripWorkspaceDisplayPrefix(content))" in ui_src
|
||||||
assert "dataset.rawText=String(displayContent).trim()" in ui_src
|
assert "const newRawText=String(displayContent).trim();" in ui_src
|
||||||
|
assert "row.dataset.rawText=newRawText;" in ui_src
|
||||||
|
|
||||||
|
|
||||||
def test_attached_files_context_is_hidden_from_sidebar_titles():
|
def test_attached_files_context_is_hidden_from_sidebar_titles():
|
||||||
|
|||||||
@@ -793,7 +793,7 @@ const row = {
|
|||||||
dataset: { msgIdx: '2', rawText: 'hello world' },
|
dataset: { msgIdx: '2', rawText: 'hello world' },
|
||||||
classList: { contains(name){ return name === 'msg-row'; } },
|
classList: { contains(name){ return name === 'msg-row'; } },
|
||||||
set innerHTML(val){ innerHTMLWriteCount++; },
|
set innerHTML(val){ innerHTMLWriteCount++; },
|
||||||
get innerHTML(){ return '<div class="msg-body">hello world</div>'; },
|
get innerHTML(){ return '<div class="msg-body">hello world</div><div class="msg-foot">same</div>'; },
|
||||||
};
|
};
|
||||||
|
|
||||||
const _recycleStash = new Map();
|
const _recycleStash = new Map();
|
||||||
@@ -806,9 +806,10 @@ let r = _msgNodeRecycleEnabled ? _recycleStash.get(rawIdx) : null;
|
|||||||
if(r && !r.classList.contains('msg-row')) r = null;
|
if(r && !r.classList.contains('msg-row')) r = null;
|
||||||
if(r){
|
if(r){
|
||||||
const newRawText = 'hello world';
|
const newRawText = 'hello world';
|
||||||
if(r.dataset.rawText !== newRawText){
|
const nextRowHtml = '<div class="msg-body">hello world</div><div class="msg-foot">same</div>';
|
||||||
|
if(r.dataset.rawText !== newRawText || r.innerHTML !== nextRowHtml){
|
||||||
r.dataset.rawText = newRawText;
|
r.dataset.rawText = newRawText;
|
||||||
r.innerHTML = 'would be set';
|
r.innerHTML = nextRowHtml;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -844,9 +845,10 @@ let r = _msgNodeRecycleEnabled ? _recycleStash.get(rawIdx) : null;
|
|||||||
if(r && !r.classList.contains('msg-row')) r = null;
|
if(r && !r.classList.contains('msg-row')) r = null;
|
||||||
if(r){
|
if(r){
|
||||||
const newRawText = 'new content';
|
const newRawText = 'new content';
|
||||||
if(r.dataset.rawText !== newRawText){
|
const nextRowHtml = '<div class="msg-body">new content</div>';
|
||||||
|
if(r.dataset.rawText !== newRawText || r.innerHTML !== nextRowHtml){
|
||||||
r.dataset.rawText = newRawText;
|
r.dataset.rawText = newRawText;
|
||||||
r.innerHTML = '<div class="msg-body">new content</div>';
|
r.innerHTML = nextRowHtml;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -862,6 +864,82 @@ console.log(JSON.stringify({
|
|||||||
assert out["updated"] is True, "innerHTML was not updated for changed content"
|
assert out["updated"] is True, "innerHTML was not updated for changed content"
|
||||||
assert out["rawText_updated"] is True, "rawText was not updated"
|
assert out["rawText_updated"] is True, "rawText was not updated"
|
||||||
|
|
||||||
|
def test_same_rawtext_but_changed_markup_updates_innerhtml(self):
|
||||||
|
"""Recycled rows must refresh when files/footer markup changes."""
|
||||||
|
source = r"""
|
||||||
|
let innerHTMLWriteCount = 0;
|
||||||
|
|
||||||
|
const row = {
|
||||||
|
dataset: { msgIdx: '2', rawText: 'same text' },
|
||||||
|
classList: { contains(name){ return name === 'msg-row'; } },
|
||||||
|
set innerHTML(val){ innerHTMLWriteCount++; this._html = val; },
|
||||||
|
get innerHTML(){ return '<div class="msg-body">same text</div><div class="msg-foot">old</div>'; },
|
||||||
|
};
|
||||||
|
|
||||||
|
const _recycleStash = new Map();
|
||||||
|
_recycleStash.set(2, row);
|
||||||
|
const _msgNodeRecycleEnabled = true;
|
||||||
|
|
||||||
|
const rawIdx = 2;
|
||||||
|
let r = _msgNodeRecycleEnabled ? _recycleStash.get(rawIdx) : null;
|
||||||
|
if(r && !r.classList.contains('msg-row')) r = null;
|
||||||
|
const newRawText = 'same text';
|
||||||
|
const nextRowHtml = '<div class="msg-body">same text</div><div class="msg-foot">new</div>';
|
||||||
|
if(r){
|
||||||
|
if(r.dataset.rawText !== newRawText || r.innerHTML !== nextRowHtml){
|
||||||
|
r.dataset.rawText = newRawText;
|
||||||
|
r.innerHTML = nextRowHtml;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log(JSON.stringify({
|
||||||
|
recycled: r === row,
|
||||||
|
innerHTML_writes: innerHTMLWriteCount,
|
||||||
|
updated: innerHTMLWriteCount === 1,
|
||||||
|
}));
|
||||||
|
"""
|
||||||
|
out = json.loads(_run_node(source))
|
||||||
|
assert out["recycled"] is True
|
||||||
|
assert out["updated"] is True, \
|
||||||
|
"same rawText with changed markup must still refresh the row"
|
||||||
|
|
||||||
|
def test_recycled_row_clears_transient_editing_flag(self):
|
||||||
|
"""Recycled rows must clear stale edit state before reuse."""
|
||||||
|
source = r"""
|
||||||
|
const row = {
|
||||||
|
dataset: { msgIdx: '2', rawText: 'same text', editing: '1' },
|
||||||
|
classList: { contains(name){ return name === 'msg-row'; } },
|
||||||
|
set innerHTML(val){ this._html = val; },
|
||||||
|
get innerHTML(){ return '<div class="msg-body">same text</div><div class="msg-foot">same</div>'; },
|
||||||
|
};
|
||||||
|
|
||||||
|
const _recycleStash = new Map();
|
||||||
|
_recycleStash.set(2, row);
|
||||||
|
const _msgNodeRecycleEnabled = true;
|
||||||
|
|
||||||
|
const rawIdx = 2;
|
||||||
|
let r = _msgNodeRecycleEnabled ? _recycleStash.get(rawIdx) : null;
|
||||||
|
if(r && !r.classList.contains('msg-row')) r = null;
|
||||||
|
const newRawText = 'same text';
|
||||||
|
const nextRowHtml = '<div class="msg-body">same text</div><div class="msg-foot">same</div>';
|
||||||
|
if(r){
|
||||||
|
delete r.dataset.editing;
|
||||||
|
if(r.dataset.rawText !== newRawText || r.innerHTML !== nextRowHtml){
|
||||||
|
r.dataset.rawText = newRawText;
|
||||||
|
r.innerHTML = nextRowHtml;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log(JSON.stringify({
|
||||||
|
recycled: r === row,
|
||||||
|
editing_cleared: !('editing' in r.dataset),
|
||||||
|
}));
|
||||||
|
"""
|
||||||
|
out = json.loads(_run_node(source))
|
||||||
|
assert out["recycled"] is True
|
||||||
|
assert out["editing_cleared"] is True, \
|
||||||
|
"recycled rows must drop stale dataset.editing state"
|
||||||
|
|
||||||
|
|
||||||
# ═══════════════════════════════════════════════════════════════════════════
|
# ═══════════════════════════════════════════════════════════════════════════
|
||||||
# Tier 5: Scrollbar drag suppression — prevent innerHTML wipe during
|
# Tier 5: Scrollbar drag suppression — prevent innerHTML wipe during
|
||||||
@@ -983,6 +1061,15 @@ console.log(JSON.stringify({
|
|||||||
assert '_scheduleMessageVirtualizedRender(true)' in handler, \
|
assert '_scheduleMessageVirtualizedRender(true)' in handler, \
|
||||||
"pointercancel handler does not trigger forced re-render"
|
"pointercancel handler does not trigger forced re-render"
|
||||||
|
|
||||||
|
def test_blur_and_visibilitychange_clear_flag(self):
|
||||||
|
"""Losing focus must not leave scrollbar drag mode stuck on."""
|
||||||
|
assert "window.addEventListener('blur'" in JS, \
|
||||||
|
"blur listener not registered for drag cleanup"
|
||||||
|
assert "document.addEventListener('visibilitychange'" in JS, \
|
||||||
|
"visibilitychange listener not registered for drag cleanup"
|
||||||
|
assert "document.visibilityState==='hidden'" in JS, \
|
||||||
|
"visibilitychange cleanup must guard on hidden state"
|
||||||
|
|
||||||
|
|
||||||
class TestScrollbarDragRenderDuringDrag:
|
class TestScrollbarDragRenderDuringDrag:
|
||||||
"""During scrollbar drag, _scheduleMessageVirtualizedRender must run full
|
"""During scrollbar drag, _scheduleMessageVirtualizedRender must run full
|
||||||
@@ -1649,6 +1736,48 @@ console.log(JSON.stringify({ updates_key: updatesKey }));
|
|||||||
assert out["updates_key"] is True, \
|
assert out["updates_key"] is True, \
|
||||||
"drag path must update _messageVirtualWindowKey"
|
"drag path must update _messageVirtualWindowKey"
|
||||||
|
|
||||||
|
def test_drag_path_defers_programmatic_scroll_clear(self):
|
||||||
|
"""The drag path must schedule a deferred clear even when delta < 2px."""
|
||||||
|
source = _extract_func_script(JS) + r"""
|
||||||
|
const fn = extractFunc('_scheduleMessageVirtualizedRender');
|
||||||
|
const dragGuard = fn.indexOf('if(_scrollbarDragActive)');
|
||||||
|
const returnIdx = fn.indexOf('return;', dragGuard);
|
||||||
|
const dragBlock = fn.slice(dragGuard, returnIdx + 10);
|
||||||
|
const schedulesClear = dragBlock.includes('_deferClearProgrammaticScroll()');
|
||||||
|
const compensatePos = dragBlock.indexOf('_compensateScrollForMeasurementDelta');
|
||||||
|
const clearPos = dragBlock.indexOf('_deferClearProgrammaticScroll()');
|
||||||
|
console.log(JSON.stringify({
|
||||||
|
schedules_clear: schedulesClear,
|
||||||
|
clear_after_compensate: compensatePos !== -1 && clearPos > compensatePos,
|
||||||
|
}));
|
||||||
|
"""
|
||||||
|
out = json.loads(_run_node(source))
|
||||||
|
assert out["schedules_clear"] is True, \
|
||||||
|
"drag path must call _deferClearProgrammaticScroll()"
|
||||||
|
assert out["clear_after_compensate"] is True, \
|
||||||
|
"drag path must defer the clear after the compensate render"
|
||||||
|
|
||||||
|
def test_recycled_assistant_turn_refreshes_role_header(self):
|
||||||
|
"""Recycled assistant turns must refresh timestamp and TPS header markup."""
|
||||||
|
source = _extract_func_script(JS) + r"""
|
||||||
|
const fn = extractFunc('renderMessages');
|
||||||
|
const assistantStart = fn.indexOf('if(!currentAssistantTurn){');
|
||||||
|
const assistantEnd = fn.indexOf('const seg=document.createElement', assistantStart);
|
||||||
|
const assistantBranch = fn.slice(assistantStart, assistantEnd);
|
||||||
|
console.log(JSON.stringify({
|
||||||
|
rewrites_role_header: assistantBranch.includes(\"role.outerHTML=_assistantRoleHtml(\"),
|
||||||
|
refreshes_session_id: assistantBranch.includes(\"currentAssistantTurn.dataset.sessionId=S.session.session_id\"),
|
||||||
|
clears_transparent_collapse: assistantBranch.includes(\"recycled.removeAttribute('data-transparent-turn-collapsed')\"),
|
||||||
|
}));
|
||||||
|
"""
|
||||||
|
out = json.loads(_run_node(source))
|
||||||
|
assert out["rewrites_role_header"] is True, \
|
||||||
|
"recycled assistant turns must refresh the role header"
|
||||||
|
assert out["refreshes_session_id"] is True, \
|
||||||
|
"recycled assistant turns must refresh the session id"
|
||||||
|
assert out["clears_transparent_collapse"] is True, \
|
||||||
|
"recycled assistant turns must clear stale transparent collapse state"
|
||||||
|
|
||||||
|
|
||||||
class TestStashKeyCoercion:
|
class TestStashKeyCoercion:
|
||||||
"""The stash uses Number(key) for storage. Verify dataset string values
|
"""The stash uses Number(key) for storage. Verify dataset string values
|
||||||
|
|||||||
+15
-4
@@ -130,10 +130,21 @@ class TestScrollPinningFix:
|
|||||||
"""Scroll listener must hide the button when user is near the bottom (#677)."""
|
"""Scroll listener must hide the button when user is near the bottom (#677)."""
|
||||||
scroll_listener_start = UI_JS.find("el.addEventListener('scroll'")
|
scroll_listener_start = UI_JS.find("el.addEventListener('scroll'")
|
||||||
assert scroll_listener_start != -1, "scroll event listener not found"
|
assert scroll_listener_start != -1, "scroll event listener not found"
|
||||||
# After #1360 fix, the nearBottom + btn logic lives inside an rAF
|
listener_open = UI_JS.find("{", scroll_listener_start)
|
||||||
# callback — extend search window to cover the full listener block.
|
assert listener_open != -1, "scroll listener body not found"
|
||||||
<<<<<<< HEAD
|
depth = 0
|
||||||
listener_block = UI_JS[scroll_listener_start:scroll_listener_start + 1800]
|
listener_end = -1
|
||||||
|
for idx in range(listener_open, len(UI_JS)):
|
||||||
|
ch = UI_JS[idx]
|
||||||
|
if ch == "{":
|
||||||
|
depth += 1
|
||||||
|
elif ch == "}":
|
||||||
|
depth -= 1
|
||||||
|
if depth == 0:
|
||||||
|
listener_end = idx + 1
|
||||||
|
break
|
||||||
|
assert listener_end != -1, "scroll listener body did not terminate cleanly"
|
||||||
|
listener_block = UI_JS[scroll_listener_start:listener_end]
|
||||||
assert "_syncScrollToBottomCue(showBottomButton" in listener_block, (
|
assert "_syncScrollToBottomCue(showBottomButton" in listener_block, (
|
||||||
"Scroll listener must show/hide scrollToBottomBtn based on _scrollPinned (#677)"
|
"Scroll listener must show/hide scrollToBottomBtn based on _scrollPinned (#677)"
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -47,4 +47,5 @@ def test_user_render_uses_stripped_display_content_without_preempting_context_ca
|
|||||||
# already-stripped displayContent, so the invariant holds either way.
|
# already-stripped displayContent, so the invariant holds either way.
|
||||||
assert ("_renderUserFencedBlocks(displayContent)" in render_prefix or
|
assert ("_renderUserFencedBlocks(displayContent)" in render_prefix or
|
||||||
"_getCachedRender(displayContent, isUser)" in render_prefix)
|
"_getCachedRender(displayContent, isUser)" in render_prefix)
|
||||||
assert "row.dataset.rawText=String(displayContent).trim();" in render_prefix
|
assert "const newRawText=String(displayContent).trim();" in render_prefix
|
||||||
|
assert "row.dataset.rawText=newRawText;" in render_prefix
|
||||||
Reference in new issue
Block a user