Open
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Accidentally committed review comment in source file
- Removed the stray internal
// REVIEW:note fromcomparemarkers.tswithout altering any functional code.
- Removed the stray internal
Or push these changes by commenting:
@cursor push 09d25c103e
Preview (09d25c103e)
diff --git a/packages/ckeditor5-engine/src/conversion/comparemarkers.ts b/packages/ckeditor5-engine/src/conversion/comparemarkers.ts
--- a/packages/ckeditor5-engine/src/conversion/comparemarkers.ts
+++ b/packages/ckeditor5-engine/src/conversion/comparemarkers.ts
@@ -3,7 +3,6 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options
*/
-// REVIEW: Let's call this file `comparemarkers.ts` (adjust module name and imports)
/**
* @module engine/conversion/comparemarkers
*/This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
niegowski
reviewed
Mar 30, 2026
| // reverse DOM order, and intersecting ranges are in something approximating | ||
| // reverse DOM order (since reverse DOM order doesn't have a precise meaning | ||
| // when working with intersecting ranges). | ||
| result.sort( compareMarkersForDowncast ); |
Contributor
There was a problem hiding this comment.
This was not needed as the markers map was already sorted in DowncastDispatcher#convert().
scofalik
reviewed
Mar 31, 2026
Comment on lines
+204
to
+215
| // Sort the markers in a stable fashion to ensure that the order in which they are | ||
| // added to the model's marker collection does not affect how they are | ||
| // downcast. One particular use case that we are targeting here, is one where | ||
| // two markers are adjacent but not overlapping, such as an insertion/deletion | ||
| // suggestion pair representing the replacement of a range of text. In this | ||
| // case, putting the markers in DOM order causes the first marker's end to be | ||
| // serialized right after the second marker's start, while putting the markers | ||
| // in reverse DOM order causes it to be right before the second marker's | ||
| // start. So, we sort these in a way that ensures non-intersecting ranges are in | ||
| // reverse DOM order, and intersecting ranges are in something approximating | ||
| // reverse DOM order (since reverse DOM order doesn't have a precise meaning | ||
| // when working with intersecting ranges). |
Contributor
There was a problem hiding this comment.
I propose to add an example, and maybe shorten the text then. It will be easier to imagine what is going on here.
scofalik
reviewed
Mar 31, 2026
Comment on lines
+1338
to
+1339
| viewWriter.setCustomProperty( 'markerBoundaryType', 'opening', viewStartElement ); | ||
| viewWriter.setCustomProperty( 'markerBoundaryType', 'closing', viewEndElement ); |
Contributor
There was a problem hiding this comment.
I propose 'start' and 'end' as values, seems simpler, and we always use these two nouns.
…nd" to better reflect used terms.
scofalik
approved these changes
Mar 31, 2026
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
3 participants
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.
🚀 Summary
Fixes incorrect order of adjacent marker boundary elements in editing downcast.
📌 Related issues
💡 Additional information
Optional: Notes on decisions, edge cases, or anything helpful for reviewers.
🧾 Checklists
Use the following checklists to ensure important areas were not overlooked.
This does not apply to feature-branch merges.
If an item is not relevant to this type of change, simply leave it unchecked.
Author checklist
Reviewer checklist
t()(if any).