[ajuncosa]

Ports: #34917
Resolves: #34898
Resolves: #22695

Instead of simply porting the 4.7 fix, this PR normalises how styled properties are written and read for all SLines:

• Each line type now writes only its unstyled properties explicitly, and SLine writes every overridden styled property in one place (writeStyledProperties).
• Reading goes through a single readStyledProperty call in readProperties(SLine) instead of scattered per-type calls and UNSTYLED flags.
• As a result, some overrides that were previously lost on save are now kept, e.g.:
  • PLACEMENT for pedal, let ring, palm mute,
  • TEXT_SIZE_SPATIUM_DEPENDENT for all text lines (doesn’t seem to be really used, and the Scale with staff size checkbox doesn’t work for lines, I think it goes through a different property, but that’s a separate issue), and
  • COLOR for gradual tempo changes.
• Overridden line properties of glissandos and the offset of text lines are no longer written twice.
• Overridden styled properties are now written after the unstyled ones, so the tag order changes in the unit tests' references.

[coderabbitai]

📝 Walkthrough

Engraving style mappings and XML property handling are updated. The changes add parameterized tests for styled-property serialization and restoration, provide a helper to serialize an engraving item, and reorder fields in XML reference data without changing their values.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: 🔵 Low · up to a52c2

Edited text-line fonts can produce duplicate XML tags on the first save. Reopening preserves the formatting, so this is a bounded issue rather than a merge blocker.

Security architecture risk: 🔵 Low · up to 22d37

The change is confined to saving and restoring score properties, and no new privileged access was identified. Risk remains low because older readers may discard newly preserved overrides, while malformed-input and recovery behavior have not been fully validated.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — Untrusted score XML reaches the existing engraving-property parser and can restore additional overrides in the opened score. The inspected added property's sink is a Boolean text-size setting followed by layout invalidation. No added privileged action was identified in this route; broader malformed-input process safety remains unverified.

Trust Boundaries and Controls

• observed — The shared reader retains property-name matching and the existing shouldSkipProperty gate before mutation. The PR changes which mapped properties reach that reader, not the gate itself. Unknown XML elements are skipped by the existing unknown-element handler.

Resilience and Maintainability Implications

• observed — Persistence still encodes value differences rather than an independent style-ownership record: an explicit override equal to its default is omitted. This limits exact ownership preservation but predates the PR, so it is not retained as an introduced architecture concern.

Pre-merge checks · 3 · 2

Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 10.87% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 46 functions across 10 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check		The description explains the motivation, implementation, affected behavior, and linked issues. However, it omits the required checklist from the repository template, including CLA, coding rules, testi…	Add the required template checklist and mark each item accurately. Check the unit-test item because this PR adds tests. Complete the AI-assistance section and list the tools used if applicable.

Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check		[#34898] The shared SLine styled-property read/write path preserves overridden pedal placement through save and reload. [#22695] The same path preserves overridden color for gradual tempo changes. `st…
Out of Scope Changes check		The serializer and reader changes, property mapping updates, round-trip tests, ScoreRW test support, and XML reference reordering all support the stated SLine styled-property serialization refactor.…
Title check		The title clearly summarizes the main change: refactoring styled-property handling for SLine elements.

Full details: Description check

Explanation

The description explains the motivation, implementation, affected behavior, and linked issues. However, it omits the required checklist from the repository template, including CLA, coding rules, testing, commit, and AI-assistance declarations.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped musescore/muse_framework.git.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[davidstephengrant]

@ajuncosa Font style and font size overrides on lines with text (rall., rit., staff text line, pedal, ottava, volta) are written twice in the saved file.

1. Open any score
2. Add e.g. rall. from the palette
3. In Properties > Text, turn on Italic
4. Save, and look at the <GradualTempoChange> element in the .mscx

Result: beginFontStyle, continueFontStyle and endFontStyle are each written twice: once with the line's other properties, and again after <placement> / <color>. In Nightly main and 4.7.5 each one is written once.

<GradualTempoChange>
  ...
  <beginText>rall.</beginText>
  <beginFontStyle>3</beginFontStyle>
  <beginTextOffset x="0" y="0"/>
  <continueText>(rall.)</continueText>
  <continueFontStyle>3</continueFontStyle>
  <endFontStyle>3</endFontStyle>
  <track2>0</track2>
  ...
  <placement>below</placement>
  <color r="224" g="27" b="36" a="255"/>
  <beginFontStyle>3</beginFontStyle>
  <continueFontStyle>3</continueFontStyle>
  <endFontStyle>3</endFontStyle>
</GradualTempoChange>

Changing the size doubles the FontSize tags too. Line width, line style, placement and colour are written once. It only happens for overrides made in the current session: reopening the file and saving again writes each tag once. I found no visible effect, as the file reopens with the correct formatting.

[davidstephengrant]

@ajuncosa Retested and approved on Windows 11 and Ubuntu 24.04.5 LTS.