HAR-9261 - Comment permissions fixes, dialog fixes#392
Merged
Conversation
harbournick
commented
Mar 19, 2025
Collaborator
- Fix missing comment header in external comments
- Allow overflow in child comments
- Fix permissions for edit/delete comment with internal/external
- Fixes comment deletion
…ild comments, fix permissions for edit/delete internal/external
superdoc-bot Bot
pushed a commit
that referenced
this pull request
Jul 9, 2026
…#392) * fix(super-editor): sync custom xml writes through part-sync - metadata.* and customXml.parts.* writes routed through the parts mutation layer so partChanged fires and part-sync publishes - a shared sandbox-and-diff wrapper commits every package path a custom xml create/update/remove touches (item, itemProps, item rels, document rels), not just the item xml - two-client regression tests cover metadata attach and customXml create/patch/remove, including the remote client's converted xml * fix(super-editor): record custom xml tombstones on remote part-sync deletes Remote part deletions removed the part from convertedXml but never populated removedCustomXmlPaths, so a collaborator whose original docx contained the part re-emitted it on export. The consumer now records tombstones for deleted custom xml storage parts, props parts, and item rels (including rels-linked props paths), and clears stale tombstones when a part is re-created remotely. * fix(super-editor): tombstone absent custom xml parts on late-join and clear bibliography cache - late joiners prune local custom xml parts absent from an authoritative parts map (a peer deleted them before this client joined), recording export tombstones through the same mutateParts mechanism hydration uses - tombstoning a path now also invalidates converter caches, so deleting the bibliography storage part clears converter.bibliographyPart and export cannot resurrect it * fix(super-editor): record hydration tombstones only after the prune mutation commits Tombstones (and the bibliography cache clear) were applied before mutateParts ran; a mutation failure rolled back the part store but left the converter tombstoned. The prune now collects paths and records them only after the mutation succeeds. * fix(super-editor): clear stale custom xml tombstones during hydration A client that tombstoned a custom xml part (e.g. deleted while offline) and then rehydrated from an authoritative map containing the recreated part kept the stale tombstone, so export dropped the recreated part. Hydration now clears tombstones for present custom xml paths at the same post-mutation point where absent paths are tombstoned - symmetric to the live consumer path. * fix(super-editor): invalidate bibliography cache on remote custom xml writes Remote writes and hydration cleared stale tombstones but left the bibliography cache pointing at the old content, so export rebuilt the part from the stale cache over the collaborator's version. Written and present custom xml paths now invalidate converter caches, and the part path predicates agree on casing. * fix(super-editor): clear hydration tombstones only for parts that hydrated A key present in the authoritative map whose envelope failed to decode never lands in convertedXml; clearing its tombstone would let export copy the stale original zip entry through instead of dropping the part. * ci: retrigger checks on current head * fix(super-editor): scope hydration tombstone clearing to hydrated parts and make converter-state application rollback-safe - hydration only clears a custom xml tombstone when the part actually hydrated this pass (a decode+op succeeded), not merely because a stale local copy is present; a failed-to-hydrate authoritative part no longer resurrects the stale local one on export - applyConverterState is atomic and runs only after the part-store mutation durably commits, so a compound rollback cannot leave removedCustomXmlPaths / bibliographyPart mutated while the store is restored * fix: address PR review comments on #392 * fix(super-editor): validate metadata payload before mutating on attach metadata.attach serialized the payload only in the live path, inside the part-sync sandbox that runs after the anchor is inserted. A non-serializable payload (a BigInt or a cyclic object, neither of which the payload type can rule out) therefore threw while building the envelope, after wrapRangeInAnchor had already dispatched the anchor transaction, leaving an inline content control with no matching customXml payload. Build the envelope during the attach preview as well, so serialization fails before the operation touches the document. Adds a regression test asserting a cyclic payload throws without dispatching an anchor transaction. * refactor(super-editor): serialize only the new entry in the attach preview The attach preview rebuilt the whole metadata envelope to validate one payload, re-serializing every existing entry in the namespace before the live path serialized them again. Serialize just the new entry — existing entries were already validated when they were written — so the preview stays a fail-fast check for non-serializable payloads without the per-attach serialization cost. * test(super-editor): fix concurrent-seed race in collaborator export test The two-client export test created both collaborative editors on unsynced ydocs, so both bootstrapped as a new room and both seeded the baseline package into their parts maps as concurrent Yjs writes. The map-key conflict on word/_rels/document.xml.rels is resolved by clientID, and with the wrong random winner the second client's baseline seed discarded the first client's published rels update, dropping the customXml document relationship from the export (~50% failure rate; deterministically reproducible by pinning clientIDs). Real clients receive room state before bootstrapping, see the capability marker, and hydrate instead of seeding, so the conflict is an artifact of the test's join order. Create the second editor only after syncing the first editor's seeded state, matching the real join flow, and wait for the received document rels content (the baseline part always exists, so an existence check passed before the remote update landed). Ported-From-Source-Repo: superdoc/orbit Ported-From-Source-Commit: 13119b74e8785850867256c4a8fffb06b41e73c3 Ported-Public-Prefix: superdoc/public
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.