feror/Sapling
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(editor): restrict viewport anchoring to presentation transitions

Browse source
This commit is contained in:
Feror 2026-06-01 15:00:55 +02:00
parent 95be4348cd
commit 183493022f
3 changed files with 235 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

35
Docs/editor-investigation.md
View file

@ -1559,6 +1559,7 @@ Root cause:
- Source code blocks reveal the whole fenced block.
- `applyHybridAttributes` restored the previous `NSClipView.bounds.origin`, but the edited glyph could move relative to that origin after attributes changed.
- Programmatic selection also called `scrollRangeToVisible` even when the selection was already visible, which could pre-scroll before the presentation transition ran.
- The first implementation applied anchoring too broadly from `applyHybridAttributes`, so ordinary selection changes and keyboard navigation could also receive custom scroll correction after AppKit had already handled caret visibility.
Corrected stabilization model:
@ -1572,11 +1573,44 @@ flowchart LR
The anchor is source-based, not element-specific. `ViewportPresentationAnchor` captures a character location and its viewport-relative Y position before a presentation transition. After styling, the coordinator asks TextKit for the same source location's new glyph position and scrolls the clip view by the delta. This applies equally to headings, task lists, code blocks, tables, and future rendered widgets because the mechanism does not inspect the rendered element type during restoration.
Anchoring policy:
```mermaid
flowchart TD
Event["Editor event"] --> Keyboard{"Keyboard navigation?"}
Keyboard -- Yes --> Native["Let NSTextView own scrolling"]
Keyboard -- No --> Selection{"Programmatic selection?"}
Selection -- Yes --> Native
Selection -- No --> Transition{"Editable region changed?"}
Transition -- No --> Native
Transition -- Yes --> Presentation{"Mouse or focus presentation transition?"}
Presentation -- Yes --> Anchor["Capture and restore presentation anchor"]
Presentation -- No --> Native
```
Viewport stabilization now runs only when all of these are true:
- A styling pass is required.
- The dirty reason is an active/editable-region transition rather than source text editing or initial render.
- The editable region actually changed.
- The selection is collapsed; range selections stay native.
- The trigger is mouse presentation entry or focus-driven presentation exit.
Viewport stabilization explicitly does not run for:
- Arrow-key navigation.
- Page up/down and other key-driven selection movement.
- Mouse drag or range selection.
- Programmatic selection updates.
- Source edits.
- View updates that do not change the editable region.
Anchor selection:
- The selected source location is the default anchor.
- If selection is on hidden or structural syntax for headings, lists, blockquotes, or tasks, the anchor moves to the visible content range for that line.
- Programmatic selection changes only scroll when the target point is outside the visible viewport; visible targets preserve the existing origin.
- `ViewportStabilityEvent` records the event cause, render reason, stabilization decision, whether an anchor was captured, and whether a scroll restoration ran. This is intentionally test-facing instrumentation so regressions show which system tried to own scroll behavior.
Geometry audit:
@ -1591,6 +1625,7 @@ Validation:
- `testHeadingTransitionKeepsEditedContentVisuallyAnchored` reproduced the heading jump and now verifies the heading text keeps the same viewport-relative Y position when entering and leaving source mode.
- `testCodeBlockTransitionKeepsEditedContentVisuallyAnchored` verifies the same contract for entering and leaving a rendered code block.
- `testKeyboardNavigationDoesNotRunViewportStabilization` verifies arrow-key-style selection movement is logged as native keyboard navigation and does not run custom scroll restoration.
- The tests use `HybridMarkdownLiveEditorHarness`, so they exercise the same `NSTextView`, coordinator, TextKit layout, and scroll-view path as the app.
Performance impact:

198
Sources/SaplingEditor/HybridMarkdownEditor.swift
View file

@ -206,10 +206,10 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
textView.string = text
textView.delegate = context.coordinator
textView.onFocusStateChange = { [weak coordinator = context.coordinator] textView in
coordinator?.applyHybridAttributes(to: textView)
coordinator?.applyHybridAttributes(to: textView, cause: .focusChange)
}
textView.onUserEditingInteraction = { [weak coordinator = context.coordinator] textView in
coordinator?.activateEditingPresentation(in: textView)
textView.onUserEditingInteraction = { [weak coordinator = context.coordinator] textView, interactionKind in
coordinator?.activateEditingPresentation(in: textView, interactionKind: interactionKind)
}
textView.isRichText = false
textView.isEditable = true
@ -255,7 +255,7 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
let selectedRange = selection.range
if textView.selectedRange() != selectedRange,
selectedRange.location <= textView.string.utf16.count {
context.coordinator.setSelection(selectedRange, in: textView)
context.coordinator.setSelection(selectedRange, in: textView, interactionKind: .programmatic)
}
context.coordinator.applyHybridAttributes(to: textView)
@ -271,6 +271,9 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
private var pendingEdit: DocumentLineIndexEdit?
private var hasUserActivatedEditing = false
private var checklistButtons: [Int: ChecklistOverlayButton] = [:]
fileprivate var viewportStabilityEvents: [ViewportStabilityEvent] = []
private var lastUserInteractionKind: EditorInteractionKind = .programmatic
private var pendingSelectionInteractionKind: EditorInteractionKind?
init(_ parent: NativeMarkdownTextView) {
self.parent = parent
@ -303,7 +306,7 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
}
parent.onTextEdit(textView.string, edit, selection)
parent.selection = selection
applyHybridAttributes(to: textView)
applyHybridAttributes(to: textView, cause: .sourceChange)
pendingEdit = nil
}
@ -312,11 +315,16 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
guard let textView = notification.object as? NSTextView else { return }
let newSelection = EditorSelection(range: textView.selectedRange())
guard parent.selection != newSelection else { return }
applyHybridAttributes(to: textView)
let interactionKind = pendingSelectionInteractionKind ?? lastUserInteractionKind
pendingSelectionInteractionKind = nil
applyHybridAttributes(to: textView, cause: .selectionChange(interactionKind))
parent.selection = newSelection
}
func applyHybridAttributes(to textView: NSTextView) {
func applyHybridAttributes(
to textView: NSTextView,
cause: PresentationUpdateCause = .viewUpdate
) {
guard let textStorage = textView.textStorage else { return }
let editableRegion = presentationEditableRegion(in: textView)
let activeLineIndex = editableRegion.primaryLineIndex
@ -324,11 +332,19 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
guard invalidationPlan.requiresStyling else { return }
let selectedRange = textView.selectedRange()
let viewportAnchor = ViewportPresentationAnchor.capture(
in: textView,
selectedRange: selectedRange,
lineIndex: currentLineIndex
let stabilizationDecision = viewportStabilizationDecision(
cause: cause,
invalidationPlan: invalidationPlan,
editableRegion: editableRegion,
selectedRange: selectedRange
)
let viewportAnchor = stabilizationDecision.shouldStabilize
? ViewportPresentationAnchor.capture(
in: textView,
selectedRange: selectedRange,
lineIndex: currentLineIndex
)
: nil
let start = Date()
var stylingResult = MarkdownTextStylingResult.empty
var didRestoreVisibleOrigin = false
@ -351,11 +367,20 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
textView.setSelectedRange(selectedRange)
}
}
didRestoreVisibleOrigin = restoreViewportAnchor(viewportAnchor, in: textView)
if stabilizationDecision.shouldStabilize {
didRestoreVisibleOrigin = restoreViewportAnchor(viewportAnchor, in: textView)
}
lastStyledText = textView.string
lastStyledActiveLineIndex = activeLineIndex
lastStyledEditableRegion = editableRegion
recordViewportStabilityEvent(
cause: cause,
invalidationPlan: invalidationPlan,
decision: stabilizationDecision,
capturedAnchor: viewportAnchor != nil,
restored: didRestoreVisibleOrigin
)
syncChecklistControls(
in: textView,
stylingResult: stylingResult,
@ -382,17 +407,26 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
return EditableRegion.selection(textView.selectedRange(), in: currentLineIndex)
}
func activateEditingPresentation(in textView: NSTextView) {
func activateEditingPresentation(
in textView: NSTextView,
interactionKind: EditorInteractionKind = .programmatic
) {
lastUserInteractionKind = interactionKind
guard !hasUserActivatedEditing else { return }
hasUserActivatedEditing = true
applyHybridAttributes(to: textView)
applyHybridAttributes(to: textView, cause: .editingActivation(interactionKind))
}
func setSelection(_ range: NSRange, in textView: NSTextView) {
func setSelection(
_ range: NSRange,
in textView: NSTextView,
interactionKind: EditorInteractionKind = .programmatic
) {
guard textView.selectedRange() != range else { return }
performProgrammaticUpdate {
let visibleOrigin = textView.enclosingScrollView?.contentView.bounds.origin
let shouldScroll = selectionNeedsScroll(range, in: textView)
pendingSelectionInteractionKind = interactionKind
textView.setSelectedRange(range)
if shouldScroll {
textView.scrollRangeToVisible(range)
@ -447,6 +481,55 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
)
}
private func viewportStabilizationDecision(
cause: PresentationUpdateCause,
invalidationPlan: EditorDirtyLineInvalidationPlan,
editableRegion: EditableRegion,
selectedRange: NSRange
) -> ViewportStabilizationDecision {
guard invalidationPlan.reason == .activeLineChange else {
return ViewportStabilizationDecision(shouldStabilize: false, reason: "not-active-line-change")
}
guard lastStyledEditableRegion != editableRegion else {
return ViewportStabilizationDecision(shouldStabilize: false, reason: "no-editable-region-transition")
}
guard selectedRange.length == 0 else {
return ViewportStabilizationDecision(shouldStabilize: false, reason: "range-selection")
}
switch cause {
case .selectionChange(.keyboard), .editingActivation(.keyboard):
return ViewportStabilizationDecision(shouldStabilize: false, reason: "native-keyboard-navigation")
case .selectionChange(.programmatic), .editingActivation(.programmatic):
return ViewportStabilizationDecision(shouldStabilize: false, reason: "programmatic-selection")
case .selectionChange(.mouse), .editingActivation(.mouse), .focusChange:
return ViewportStabilizationDecision(shouldStabilize: true, reason: "presentation-transition")
case .selectionChange(.paste), .editingActivation(.paste), .sourceChange, .viewUpdate:
return ViewportStabilizationDecision(shouldStabilize: false, reason: "non-presentation-navigation")
}
}
private func recordViewportStabilityEvent(
cause: PresentationUpdateCause,
invalidationPlan: EditorDirtyLineInvalidationPlan,
decision: ViewportStabilizationDecision,
capturedAnchor: Bool,
restored: Bool
) {
viewportStabilityEvents.append(ViewportStabilityEvent(
cause: cause.description,
renderReason: "\(invalidationPlan.reason)",
decision: decision.reason,
capturedAnchor: capturedAnchor,
restored: restored
))
if viewportStabilityEvents.count > 200 {
viewportStabilityEvents.removeFirst(viewportStabilityEvents.count - 200)
}
}
private func syncChecklistControls(
in textView: NSTextView,
stylingResult: MarkdownTextStylingResult,
@ -630,6 +713,49 @@ private struct NativeMarkdownTextView: NSViewRepresentable {
}
}
private enum EditorInteractionKind: String {
case mouse
case keyboard
case paste
case programmatic
}
private enum PresentationUpdateCause: CustomStringConvertible {
case editingActivation(EditorInteractionKind)
case selectionChange(EditorInteractionKind)
case focusChange
case sourceChange
case viewUpdate
var description: String {
switch self {
case .editingActivation(let kind):
return "editingActivation:\(kind.rawValue)"
case .selectionChange(let kind):
return "selectionChange:\(kind.rawValue)"
case .focusChange:
return "focusChange"
case .sourceChange:
return "sourceChange"
case .viewUpdate:
return "viewUpdate"
}
}
}
private struct ViewportStabilizationDecision {
var shouldStabilize: Bool
var reason: String
}
private struct ViewportStabilityEvent: Hashable, Sendable {
var cause: String
var renderReason: String
var decision: String
var capturedAnchor: Bool
var restored: Bool
}
private struct CodeBlockContainerPresentation: Equatable {
var codeBlock: RenderedCodeBlockElement
var languageLabel: String
@ -693,7 +819,7 @@ private struct ViewportPresentationAnchor {
private final class EditorTextView: NSTextView {
var onFocusStateChange: ((NSTextView) -> Void)?
var onUserEditingInteraction: ((NSTextView) -> Void)?
var onUserEditingInteraction: ((NSTextView, EditorInteractionKind) -> Void)?
var codeBlockContainers: [CodeBlockContainerPresentation] = [] {
didSet {
guard oldValue != codeBlockContainers else { return }
@ -722,17 +848,17 @@ private final class EditorTextView: NSTextView {
}
override func mouseDown(with event: NSEvent) {
onUserEditingInteraction?(self)
onUserEditingInteraction?(self, .mouse)
super.mouseDown(with: event)
}
override func keyDown(with event: NSEvent) {
onUserEditingInteraction?(self)
onUserEditingInteraction?(self, .keyboard)
super.keyDown(with: event)
}
override func paste(_ sender: Any?) {
onUserEditingInteraction?(self)
onUserEditingInteraction?(self, .paste)
super.paste(sender)
}
@ -987,10 +1113,10 @@ public final class HybridMarkdownLiveEditorHarness {
self.textView.isVerticallyResizable = true
self.textView.isHorizontallyResizable = false
self.textView.onFocusStateChange = { [weak coordinator] textView in
coordinator?.applyHybridAttributes(to: textView)
coordinator?.applyHybridAttributes(to: textView, cause: .focusChange)
}
self.textView.onUserEditingInteraction = { [weak coordinator] textView in
coordinator?.activateEditingPresentation(in: textView)
self.textView.onUserEditingInteraction = { [weak coordinator] textView, interactionKind in
coordinator?.activateEditingPresentation(in: textView, interactionKind: interactionKind)
}
self.textView.string = source
self.textView.delegate = coordinator
@ -1041,8 +1167,20 @@ public final class HybridMarkdownLiveEditorHarness {
}
public func setSelection(_ range: NSRange) {
coordinator.activateEditingPresentation(in: textView)
coordinator.setSelection(range, in: textView)
setSelection(range, interactionKind: .programmatic)
}
public func setSelectionByKeyboard(_ range: NSRange) {
setSelection(range, interactionKind: .keyboard)
}
public func setSelectionByMouse(_ range: NSRange) {
setSelection(range, interactionKind: .mouse)
}
private func setSelection(_ range: NSRange, interactionKind: EditorInteractionKind) {
coordinator.activateEditingPresentation(in: textView, interactionKind: interactionKind)
coordinator.setSelection(range, in: textView, interactionKind: interactionKind)
coordinator.textViewDidChangeSelection(Notification(name: NSTextView.didChangeSelectionNotification, object: textView))
syncState()
}
@ -1109,6 +1247,18 @@ public final class HybridMarkdownLiveEditorHarness {
return CGPoint(x: point.x - origin.x, y: point.y - origin.y)
}
public func viewportStabilityEventDescriptions() -> [String] {
coordinator.viewportStabilityEvents.map {
[
"cause=\($0.cause)",
"reason=\($0.renderReason)",
"decision=\($0.decision)",
"captured=\($0.capturedAnchor)",
"restored=\($0.restored)"
].joined(separator: "|")
}
}
public func presentationSignature() -> String {
guard let storage = textView.textStorage else { return "" }
return MarkdownPresentationSnapshot.make(

28
Tests/SaplingEditorTests/HybridMarkdownLiveEditorHarnessTests.swift
View file

@ -186,7 +186,7 @@ final class HybridMarkdownLiveEditorHarnessTests: XCTestCase {
harness.scrollViewport(toY: headingPoint.y - 160)
let renderedViewportPoint = try XCTUnwrap(harness.viewportPoint(for: "Navigation"))
harness.setSelection(NSRange(location: nsSource.range(of: "Navigation").location, length: 0))
harness.setSelectionByMouse(NSRange(location: nsSource.range(of: "Navigation").location, length: 0))
let sourceViewportPoint = try XCTUnwrap(harness.viewportPoint(for: "Navigation"))
XCTAssertEqual(sourceViewportPoint.y, renderedViewportPoint.y, accuracy: 0.5)
@ -216,7 +216,7 @@ final class HybridMarkdownLiveEditorHarnessTests: XCTestCase {
harness.scrollViewport(toY: codePoint.y - 180)
let renderedViewportPoint = try XCTUnwrap(harness.viewportPoint(for: "value"))
harness.setSelection(NSRange(location: nsSource.range(of: "value").location, length: 0))
harness.setSelectionByMouse(NSRange(location: nsSource.range(of: "value").location, length: 0))
let sourceViewportPoint = try XCTUnwrap(harness.viewportPoint(for: "value"))
XCTAssertEqual(sourceViewportPoint.y, renderedViewportPoint.y, accuracy: 0.5)
@ -226,6 +226,30 @@ final class HybridMarkdownLiveEditorHarnessTests: XCTestCase {
let restoredViewportPoint = try XCTUnwrap(harness.viewportPoint(for: "value"))
XCTAssertEqual(restoredViewportPoint.y, renderedViewportPoint.y, accuracy: 0.5)
}
func testKeyboardNavigationDoesNotRunViewportStabilization() throws {
let intro = (0..<24).map { "Intro paragraph \($0)" }.joined(separator: "\n")
let outro = (0..<24).map { "Outro paragraph \($0)" }.joined(separator: "\n")
let source = """
\(intro)
# Navigation Checklist
Body text below the heading.
\(outro)
"""
let nsSource = source as NSString
let harness = HybridMarkdownLiveEditorHarness(source: source, initialWidth: 700)
harness.simulateLaunchFirstResponder()
let headingPoint = try XCTUnwrap(harness.point(for: "Navigation"))
harness.scrollViewport(toY: headingPoint.y - 160)
harness.setSelectionByKeyboard(NSRange(location: nsSource.range(of: "Navigation").location, length: 0))
XCTAssertTrue(harness.viewportStabilityEventDescriptions().contains {
$0.contains("cause=selectionChange:keyboard")
&& $0.contains("decision=native-keyboard-navigation")
&& $0.contains("restored=false")
})
}
}
#endif