fix a rather annoying collaboration behavior

Author: Titanium2099

src/addons/addons/collaboration/helpers/blocklyApplier.js

@@ -1,4 +1,5 @@
 import * as constants from './constants.js';
+import * as recorder from './recorder.js';
 
 /*
 Reconciles individual blocks in the live Blockly workspace against the VM
@@ -8,6 +9,11 @@ hits anything it can't safely patch so the caller can fall back to a full
 vm.emitWorkspaceUpdate() rebuild.
 */
 
+function bail(where, blockId, extra) {
+    recorder.record('applier.bail', Object.assign({where, block: blockId}, extra || {}));
+    return false;
+}
+
 function isObscuredShadow(target, vmBlock, blockId) {
     if (!vmBlock.shadow) return false;
     const parent = vmBlock.parent ? target.blocks.getBlock(vmBlock.parent) : null;
@@ -173,10 +179,10 @@ function reconcileState(Blockly, workspace, target, blockId) {
         return !!vmBlock.shadow;
     }
 
-    if (!!vmBlock.shadow !== wsBlock.isShadow()) return false;
+    if (!!vmBlock.shadow !== wsBlock.isShadow()) return bail('shadowness', blockId);
 
-    if (!reconcileFields(target, vmBlock, wsBlock)) return false;
-    if (!reconcileConnection(workspace, target, vmBlock, wsBlock, blockId)) return false;
+    if (!reconcileFields(target, vmBlock, wsBlock)) return bail('fields', blockId);
+    if (!reconcileConnection(workspace, target, vmBlock, wsBlock, blockId)) return bail('connection', blockId);
 
     if (vmBlock.topLevel && !wsBlock.getParent()) {
         const x = Number(vmBlock.x);
@@ -198,15 +204,100 @@ export function reconcileBlocks(target, blockIds) {
     Blockly.Events.disable();
     try {
         for (const blockId of blockIds) {
-            if (!reconcileExistence(Blockly, workspace, target, blockId)) return false;
+            if (!reconcileExistence(Blockly, workspace, target, blockId)) return bail('existence', blockId);
         }
         for (const blockId of blockIds) {
             if (!reconcileState(Blockly, workspace, target, blockId)) return false;
         }
         return true;
     } catch (e) {
         if (constants.debugging) console.warn('Collaboration: incremental block sync failed, falling back to full refresh.', e);
-        return false;
+        return bail('threw', null, {message: String(e && e.message || e)});
+    } finally {
+        Blockly.Events.enable();
+    }
+}
+
+function commentText(wsComment) {
+    if (typeof wsComment.getText === 'function') return wsComment.getText();
+    if (typeof wsComment.getContent === 'function') return wsComment.getContent();
+    return null;
+}
+
+function commentMinimized(wsComment) {
+    if (typeof wsComment.isMinimized === 'function') return !!wsComment.isMinimized();
+    return !!wsComment.isMinimized_;
+}
+
+function createComment(Blockly, workspace, vmComment) {
+    if (vmComment.blockId) {
+        const block = workspace.getBlockById(vmComment.blockId);
+        if (!block) return null;
+        if (block.comment) return block.comment.id === vmComment.id ? block.comment : null;
+        block.setCommentText(String(vmComment.text == null ? '' : vmComment.text), vmComment.id,
+            Number(vmComment.x), Number(vmComment.y), !!vmComment.minimized);
+        return block.comment || null;
+    }
+    if (typeof vmComment.toXML !== 'function') return null;
+    const dom = Blockly.Xml.textToDom(`<xml>${vmComment.toXML()}</xml>`);
+    const commentDom = dom.firstElementChild || dom.firstChild;
+    if (!commentDom) return null;
+    return Blockly.WorkspaceCommentSvg.fromXml(commentDom, workspace);
+}
+
+export function reconcileComments(target, commentIds) {
+    const Blockly = constants.mutableRefs.BlocklyInstance;
+    const workspace = constants.editorWorkspace();
+    if (!workspace || !target || !target.comments) return false;
+    // The VM holds a comment's position as an LTR workspace reports it; a mirrored workspace is
+    // rebuilt, as it was.
+    if (workspace.RTL) return bail('comments-rtl', null);
+    Blockly.Events.disable();
+    try {
+        for (const id of commentIds) {
+            const vmComment = target.comments[id];
+            let wsComment = workspace.getCommentById(id);
+            if (!vmComment) {
+                if (wsComment) wsComment.dispose();
+                continue;
+            }
+            if (!wsComment) {
+                wsComment = createComment(Blockly, workspace, vmComment);
+                if (!wsComment) return bail('comment-create', id);
+            }
+            if (vmComment.blockId) {
+                const block = workspace.getBlockById(vmComment.blockId);
+                if (!block || block.comment !== wsComment) return bail('comment-block', id);
+            }
+
+            const text = String(vmComment.text == null ? '' : vmComment.text);
+            // Not while this person is typing in it: their text is published when they blur, and
+            // the room settles it then. Overwriting the textarea under them is the report.
+            const typingHere = !!wsComment.textarea_ && document.activeElement === wsComment.textarea_;
+            if (!typingHere && commentText(wsComment) !== text) wsComment.setText(text);
+
+            const minimized = !!vmComment.minimized;
+            if (commentMinimized(wsComment) !== minimized) wsComment.setMinimized(minimized);
+
+            const width = Number(vmComment.width);
+            const height = Number(vmComment.height);
+            if (!minimized && isFinite(width) && isFinite(height)) {
+                const size = wsComment.getHeightWidth();
+                if (Math.abs(size.width - width) > 0.5 || Math.abs(size.height - height) > 0.5) {
+                    wsComment.setSize(width, height);
+                }
+            }
+
+            const x = Number(vmComment.x);
+            const y = Number(vmComment.y);
+            if (isFinite(x) && isFinite(y)) {
+                const xy = wsComment.getXY();
+                if (Math.abs(xy.x - x) > 0.5 || Math.abs(xy.y - y) > 0.5) wsComment.moveTo(x, y);
+            }
+        }
+        return true;
+    } catch (e) {
+        return bail('comments-threw', null, {message: String(e && e.message || e)});
     } finally {
         Blockly.Events.enable();
     }

src/addons/addons/collaboration/helpers/observeHandlers.js

@@ -321,33 +321,42 @@ export function sharedMonitors (event) {
     }
 }
 
-export function sharedComments (event){
-    let needsWorkspaceRefresh = false;
+/*
+Applies a comments change to the VM and says which comments of which target it touched, so the
+scheduler can patch exactly those on the workspace (blocklyApplier.reconcileComments) instead of
+rebuilding it.
+*/
+export function sharedComments (event) {
+    const touched = new Map();
+    const note = (targetId, commentId) => {
+        if (!touched.has(targetId)) touched.set(targetId, []);
+        touched.get(targetId).push(commentId);
+    };
     const path = event.path;
 
     if (path.length === 0) {
         event.changes.keys.forEach((change, targetId) => {
-            if (change.action === 'add') {
-                const target = constants.mutableRefs.vm.runtime.getTargetById(targetId);
-                if (!target) return;
-                const yTargetCommentMap = event.target.get(targetId);
-                yTargetCommentMap.forEach((yCommentMap, commentId) => {
-                    const commentData = helper.deserializeCommentFromYjs(yCommentMap);
-                    if (!target.comments[commentId]) {
-                        target.createComment(
-                            commentData.id, commentData.blockId, commentData.text,
-                            commentData.x, commentData.y, commentData.width,
-                            commentData.height, commentData.minimized, true
-                        );
-                        needsWorkspaceRefresh = true;
-                    }
-                });
-            }
+            if (change.action === 'delete') return;
+            const target = constants.mutableRefs.vm.runtime.getTargetById(targetId);
+            if (!target) return;
+            const yTargetCommentMap = event.target.get(targetId);
+            if (!yTargetCommentMap) return;
+            yTargetCommentMap.forEach((yCommentMap, commentId) => {
+                const commentData = helper.deserializeCommentFromYjs(yCommentMap);
+                if (!target.comments[commentId]) {
+                    target.createComment(
+                        commentData.id, commentData.blockId, commentData.text,
+                        commentData.x, commentData.y, commentData.width,
+                        commentData.height, commentData.minimized, true
+                    );
+                    note(targetId, commentId);
+                }
+            });
         });
     } else if (path.length === 1) {
         const targetId = path[0];
         const target = constants.mutableRefs.vm.runtime.getTargetById(targetId);
-        if (!target) return needsWorkspaceRefresh;
+        if (!target) return touched;
 
         event.changes.keys.forEach((change, commentId) => {
             if (change.action === 'add' || change.action === 'update') {
@@ -374,17 +383,17 @@ export function sharedComments (event){
                 }
                 delete target.comments[commentId];
             }
+            note(targetId, commentId);
         });
-        needsWorkspaceRefresh = true;
     } else if (path.length === 2) {
         const [targetId, commentId] = path;
         const target = constants.mutableRefs.vm.runtime.getTargetById(targetId);
-        if (!target || !target.comments[commentId]) return needsWorkspaceRefresh;
+        if (!target || !target.comments[commentId]) return touched;
 
         event.changes.keys.forEach((change, key) => {
             target.comments[commentId][key] = event.target.get(key);
         });
-        needsWorkspaceRefresh = true;
+        note(targetId, commentId);
     }
-    return needsWorkspaceRefresh;
+    return touched;
 }

src/addons/addons/collaboration/helpers/refreshScheduler.js

@@ -1,12 +1,14 @@
 import * as constants from './constants.js';
 import * as applier from './blocklyApplier.js';
 import * as helper from './helper.js';
+import * as recorder from './recorder.js';
 
 const FLUSH_DELAY_MS = 80;
 const MAX_WAIT_MS = 600;
 const DRAG_RETRY_MS = 200;
 
 const dirtyBlocksByTarget = new Map();
+const dirtyCommentsByTarget = new Map();
 const repairPending = new Set();
 let fullRefreshPending = false;
 let toolboxRefreshPending = false;
@@ -17,13 +19,55 @@ function getWorkspace() {
     return constants.editorWorkspace();
 }
 
+function holdInHand(Blockly, workspace) {
+    const held = {selectedId: Blockly.selected ? Blockly.selected.id : null, field: null};
+    const owner = Blockly.WidgetDiv.owner_;
+    const input = Blockly.FieldTextInput ? Blockly.FieldTextInput.htmlInput_ : null;
+    if (Blockly.WidgetDiv.isVisible() && input && owner && owner.sourceBlock_ && owner.name &&
+        owner.sourceBlock_.workspace === workspace) {
+        held.field = {
+            blockId: owner.sourceBlock_.id,
+            name: owner.name,
+            value: input.value,
+            start: input.selectionStart,
+            end: input.selectionEnd
+        };
+    }
+    return held;
+}
+
+function giveBack(Blockly, workspace, held) {
+    if (!workspace || !held) return;
+    if (held.selectedId && (!Blockly.selected || Blockly.selected.id !== held.selectedId)) {
+        const block = workspace.getBlockById(held.selectedId);
+        if (block && typeof block.select === 'function') block.select();
+    }
+    if (!held.field) return;
+    const block = workspace.getBlockById(held.field.blockId);
+    const field = block ? block.getField(held.field.name) : null;
+    if (!field || typeof field.showEditor_ !== 'function' || Blockly.WidgetDiv.isVisible()) return;
+    try {
+        field.showEditor_();
+        const input = Blockly.FieldTextInput.htmlInput_;
+        if (!input) return;
+        if (input.value !== held.field.value) input.value = held.field.value;
+        const length = input.value.length;
+        const start = Math.min(held.field.start == null ? length : held.field.start, length);
+        const end = Math.min(held.field.end == null ? length : held.field.end, length);
+        input.setSelectionRange(start, end);
+    } catch (e) {
+    }
+}
+
 export function rebuildWorkspace() {
     const vm = constants.mutableRefs.vm;
     if (!vm) return;
+    const Blockly = constants.mutableRefs.BlocklyInstance;
     const workspace = getWorkspace();
     const targetBefore = vm.editingTarget ? vm.editingTarget.id : null;
     const undoStack = workspace ? workspace.undoStack_.slice() : null;
     const redoStack = workspace ? workspace.redoStack_.slice() : null;
+    const held = (workspace && Blockly) ? holdInHand(Blockly, workspace) : null;
 
     vm.emitWorkspaceUpdate();
 
@@ -32,6 +76,7 @@ export function rebuildWorkspace() {
         workspace.undoStack_ = undoStack;
         workspace.redoStack_ = redoStack;
     }
+    if (held && targetAfter === targetBefore) giveBack(Blockly, getWorkspace(), held);
 }
 
 function claimsChild(parent, childId) {
@@ -82,7 +127,25 @@ export function queueBlockSync(targetId, blockIds) {
     arm();
 }
 
+export function queueCommentSync(targetId, commentIds) {
+    if (!targetId || !commentIds || commentIds.length === 0) return;
+    const editingTarget = constants.mutableRefs.vm?.editingTarget;
+    if (!editingTarget || editingTarget.id !== targetId) return;
+    let set = dirtyCommentsByTarget.get(targetId);
+    if (!set) {
+        set = new Set();
+        dirtyCommentsByTarget.set(targetId, set);
+    }
+    commentIds.forEach(id => set.add(id));
+    arm();
+}
+
 export function queueFullRefresh() {
+    if (!fullRefreshPending && recorder.isRecording()) {
+        recorder.record('refresh.queued', {
+            via: new Error().stack.split('\n').slice(2, 5).map(line => line.trim()).join(' < ')
+        });
+    }
     fullRefreshPending = true;
     arm();
 }
@@ -113,6 +176,7 @@ function flush() {
     }
 
     firstQueuedAt = 0;
+    const why = {queued: fullRefreshPending, repairs: repairPending.size, attachments: false, applier: false, comments: false};
     let needFull = fullRefreshPending;
     fullRefreshPending = false;
     const toRepair = [...repairPending];
@@ -121,6 +185,8 @@ function flush() {
     toolboxRefreshPending = false;
     const dirty = new Map(dirtyBlocksByTarget);
     dirtyBlocksByTarget.clear();
+    const dirtyComments = new Map(dirtyCommentsByTarget);
+    dirtyCommentsByTarget.clear();
 
     const previousGroup = Blockly.Events.getGroup();
     Blockly.Events.setGroup('yjs-remote-sync');
@@ -143,14 +209,28 @@ function flush() {
         if (!needFull && dirty.size > 0 && editingTarget) {
             const dirtyIds = dirty.get(editingTarget.id);
             if (dirtyIds && dirtyIds.size > 0) {
-                if (attachmentsDisagree(editingTarget, dirtyIds) ||
-                    !applier.reconcileBlocks(editingTarget, dirtyIds)) {
+                if (attachmentsDisagree(editingTarget, dirtyIds)) {
+                    why.attachments = true;
+                    needFull = true;
+                } else if (!applier.reconcileBlocks(editingTarget, dirtyIds)) {
+                    why.applier = true;
                     needFull = true;
                 }
             }
         }
 
+        if (!needFull && dirtyComments.size > 0 && editingTarget) {
+            const commentIds = dirtyComments.get(editingTarget.id);
+            if (commentIds && commentIds.size > 0 && !applier.reconcileComments(editingTarget, commentIds)) {
+                why.comments = true;
+                needFull = true;
+            }
+        }
+
         if (needFull && editingTarget) {
+            recorder.record('refresh.full', Object.assign(why, {
+                dirty: editingTarget && dirty.get(editingTarget.id) ? [...dirty.get(editingTarget.id)] : []
+            }));
             const repairedByTarget = new Map();
             vm.runtime.targets.forEach(target => {
                 if (!target.isOriginal) return;
@@ -187,4 +267,5 @@ export function reset() {
     fullRefreshPending = false;
     toolboxRefreshPending = false;
     dirtyBlocksByTarget.clear();
+    dirtyCommentsByTarget.clear();
 }

src/addons/addons/collaboration/userscript.js

@@ -455,12 +455,12 @@ function attachYjsProvider() {
                         return;
                     }
 
-                    if (OH.sharedComments(event)) {
-                        const editingTargetId = constants.mutableRefs.vm.editingTarget?.id;
-                        if (!targetId || targetId === editingTargetId) {
-                            scheduler.queueFullRefresh();
+                    const editingTargetId = constants.mutableRefs.vm.editingTarget?.id;
+                    OH.sharedComments(event).forEach((commentIds, touchedTargetId) => {
+                        if (touchedTargetId === editingTargetId) {
+                            scheduler.queueCommentSync(touchedTargetId, commentIds);
                         }
-                    }
+                    });
                 });
             } catch (e) {
                 console.warn('[collaboration] comment sync failed', e);